Skip to content

Conversation

@ismaelsadeeq
Copy link
Member

@ismaelsadeeq ismaelsadeeq commented Dec 15, 2025

This PR is another attempt to fix #27995 using a better approach.

For background and motivation, see #27995 and the discussion in the Delving Bitcoin post Mempool Based Fee Estimation on Bitcoin Core
This PR is currently limited to using the mempool only to lower what is recommended by the Block Policy Estimator.
Accurate and safe fee estimation using the mempool is challenging. There are open questions about how to prevent mempool games that are theoretically possible for miners (a variant of the Finney attack)

This is one of the reasons I opted to use the mempool only to lower the Block Policy Estimator, which itself is not gameable, and therefore this mechanism is not susceptible to this attack.

The underlying assumption here is that, with the current tools and work done to make RBF and CPFP feasible and guarantee (TRUC transaction relay, ephemeral anchors, cluster size 2 package rbf), it is safer to underestimate rather than overestimate.
We now assume it is relatively easy to fee-bump later if a transaction does not confirm, whereas once a fee is overestimated there is no way to recover from that.

Another open question when using the mempool for fee estimation is how to account for incoming transaction inflow.
Bitcoin Augur does this by using past inflow plus a constant expected inflow to predict future inflow. I find this unconvincing for fee estimation and potentially prone to more overestimation, as past conditions are not always representative of the future. See my review of the Augur fee rate estimator and open questions.

This PR uses a much simpler approach based on current user behavior, similar to the widely used method employed by mempool.space: looking at the top block of the mempool and selecting a percentile feerate depending on whether the user is economical or conservative.

Empirical data from both myself and Clara Shikhelman shows that the 75th percentile feerate for economical users and the 50th percentile feerate for conservative users provide positive confirmation guarantees, hence this is what is used in this PR.

Parallel research by René Pickhardt and his student suggests that using the average fee per byte of the block template performs well.

All of these are constants that can be adjusted, there is parallel work exploring these constants and running benchmarks across fee estimators to find a sweet spot.

See also work in LND, the LND Budget Sweeper, which applies this idea successfully. Their approach is to estimate fees initially with bitcoind, then increment gradually as the confirmation deadline approaches, using a fixed fee budget.

Historical data indicates that by doing only the approach in this PR, we are able to reduce overestimation quite significantly (~29%).

This is particularly useful in scenarios where the Block Policy Estimator recommends a high feerate while the mempool is empty.
56f3ba26c0184521c42bb82ec9d8c9f2224d4f8e

As seen in the image above, there is only one remaining unfixed case: when there is a sudden inflow of transactions and the feerate rises, the Block Policy Estimator takes time to reflect this. In that case, users will continue to see a low feerate estimate until it slowly updates. From the historical data linked above, this occurs about ~26% of the time).

Overall, we observe a 73% success rate with 0% overestimation, and 26% underestimation with this approach.

This PR also includes refactors that enable this work. Rather than splitting the PR and implementing changes incrementally, I opted for an end-to-end implementation:

1. Refactors

  • Separation of feerate format from the fee estimate mode enumerator (separation of concerns, as they are semantically distinct)
  • Move logging after fee estimation estimateSmartFee from the wallet to the Block Policy Estimator internal method (also separation of concerns and avoiding leaking fee estimator internals). This change also comes with updating createTransactionInternal to log only the fee and its source (fee estimator, fallback fee, etc.), which is more appropriate.
  • Separate the fee estimate mode enum from the fee source enumerator (separation of concerns, as they are semantically distinct). This introduces a minor breaking change: the wallet RPCs now return fee source instead of fee reason, which is more accurate. Let me know if we prefer to keep the field name unchanged while returning the fee source.
  • Move the StringForReason enum from common to the Block Policy Estimator (also separation of concerns)
  • Various test file updates and renames for improved accuracy
  • Update Block Policy Estimator unit tests to be independent of the mempool and the validation interface

2. Introduce Mempool-Based Fee Estimator and Fee Estimator Manager

  • Introduce a new abstract fee estimator base class that all fee estimators must implement

  • Introduce FeeRateEstimateResult as the response type, containing metadata and avoiding the use of out-parameters. This is easily extensible and makes future changes less invasive, as method signatures do not need to change.

  • Add FeeEstimatorType enum to identify different fee estimators

  • Make CBlockPolicyEstimator derive from the new fee estimator base class

  • Add FeeRateEstimatorManager, responsible for managing all fee estimators

  • Update the node context to store a unique_ptr to FeeRateEstimatorManager instead of CBlockPolicyEstimator

  • Update CBlockPolicyEstimator to no longer subscribe directly to the validation interface; instead, FeeRateEstimatorManager subscribes and forwards relevant notifications

  • Add a percentile calculation function that takes the chunk feerates of a block template and returns feerate percentiles; this can also be reused by the getblockstats RPC

  • Add a mempool fee estimator that generates a block template when called, calculates a percentile feerate, and returns the 75th percentile for economical mode or the 50th percentile for conservative mode

  • Add caching to the mempool estimator so that new estimates are generated only every 7 seconds, assuming enough transactions have propagated to make a meaningful difference.
    This heuristic will likely be replaced by requesting block templates via the general-purpose block template cache proposed here: RFC: Bitcoin Core Node BlockTemplateManager #33389

  • Update MempoolTransactionRemovedForBlock to return the actual block transactions as well

  • Track the weight of block transactions and evicted transactions after each block connection
    This data is tracked for the last 6 mined blocks. A mempool feerate estimate is returned only when the average ratio of mempool transaction weight evicted due to BLOCK reasons to block transaction weight is greater than 75%. This heuristic provides rough confidence that the node’s mempool matches that of the majority of the hashrate. The 75% threshold is arbitrary and can be adjusted.

There is a caveat when transactions in the local mempool are consistently not mined by the network, as described in #27995 (e.g., due to filtering).
Accounting for these transactions during fee estimation is not necessary, as they should be evicted from the mempool itself (see #33510). Handling this again within fee estimation would be redundant.

  • Persist statistics for the 6 most recent blocks to disk during periodic flushes and shutdown, so this data is available after restarts
  • Expose this data to users when running estimatesmartfee with verbosity > 2
see example output
bitcoin-cli estimatesmartfee 1 "conservative" 2
{
  "feerate": 0.00002133,
  "errors": [],
  "blocks": 2,
  "mempool_health_statistics": [
    {
      "block_height": 927953,
      "block_weight": 3991729,
      "mempool_txs_weight": 3942409,
      "ratio": 0.987644451815241
    },
    ...
  ]
}

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 15, 2025

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34075.

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:

  • #33199 (fees: enable CBlockPolicyEstimator return sub 1 sat/vb fee rate estimates by ismaelsadeeq)
  • #32427 ((RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store by sedited)
  • #32138 (wallet, rpc: remove settxfee and paytxfee by polespinasa)
  • #31382 (kernel: Flush in ChainstateManager destructor by sedited)
  • #24539 (Add a "tx output spender" index by sstone)

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.

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

  • std::accumulate(block_txs.begin() + 1, block_txs.end(), 0u, ...) in src/policy/fees/mempool_estimator.cpp
  • std::accumulate(txs_removed_for_block.begin(), txs_removed_for_block.end(), 0u, ...) in src/policy/fees/mempool_estimator.cpp

Possible places where comparison-specific test macros should replace generic comparisons:

  • test/functional/feature_fee_estimation.py:
    snippet: "assert block_stat['block_weight']" -> recommendation: replace with a comparison helper such as assert_greater_than(block_stat['block_weight'], 0)
  • test/functional/feature_fee_estimation.py:
    snippet: "assert block_stat['mempool_txs_weight']" -> recommendation: replace with a comparison helper such as assert_greater_than(block_stat['mempool_txs_weight'], 0)

No other added Python lines contain bare assert comparisons that should be replaced with the provided test helpers.

2026-01-12

@gmaxwell
Copy link
Contributor

I think this is a good idea, especially if it there was a 'very conservative' that just didn't use this.

Accounting for these transactions during fee estimation is not necessary, as they should be evicted from the mempool itself

I don't agree here-- rather time based expiration should probably be eliminated over time. If there really is a censored transaction then we want nodes to work on getting it mined, and not just give up if the censors manage to control the blocks for a long time.

Also two weeks is already long time to allow a non-minable transaction to distort fee estimates. And it's a short time for low rate confirmation-- there have been times in history when transactions weren't confirmed for months after their initial announcement (though those were also at time so far where size based expiration would have eliminated them).

So I suggest instead that future work (not this PR) could improve the estimator by excluding from considerations transactions that should have been mined according to our template but weren't for more than a threshold number of blocks, perhaps 6 or something like that.

Such code could also be used to log/warn the user that there is either ongoing censorship or that their node's policy appears to be less restrictive than the network.

But in any case it doesn't need to be done immediate to make use of this kind of estimator.

Overall, we observe a 73% success rate with 0% overestimation, and 26% underestimation with this approach.

One thing to keep in mind is that if this approach is widely used the success rate will probably change. It might make sense to initially deploy with more conservative settings (/defaults) and slowly increase them to avoid causing an abrupt change.

@ismaelsadeeq
Copy link
Member Author

I don't agree here; rather, time-based expiration should probably be eliminated over time. If there really is a censored transaction, then we want nodes to continue working to get it mined, not just give up if censors manage to control blocks for a long time.

Indeed. In the context of censorship, we would want to keep such transactions if they are incentive compatible. However, there may also be legitimate reasons miners deliberately exclude them for example, a legacy node that is unaware of a soft fork that invalidates a certain spend (e.g., post-quantum).

Hence as you also mentioned, I would like to see the direction of #33510, leaning toward deferring this improvement to future work.

Especially if there were a “very conservative” option that simply didn’t use this. One thing to keep in mind is that if this approach is widely used, the success rate will probably change. It might make sense to initially deploy with more conservative settings (or defaults) and slowly increase them to avoid causing an abrupt change.

I considered this and even had a branch that added an option to estimatesmartfee to force block-policy-only behavior.

However, the users this targets are those running bitcoind with default settings and trusting developers to make sane default choices. I’ve seen this scenario before with previous conservative defaults: complaints and issues accumulated over time, eventually leading us to switch block policy the default to economical, which provided a significant reduction in overestimation and better accuracy.

FWIW, I also did some research over a short block interval to get a sense of what Bitcoin core wallet users uses for feerate estimation, and I find that the majority of Bitcoin Core wallet users (that I was able to fingerprints) are not using the block-policy estimator, we’ve seen self-hosted users 1 2 switch approaches in the past, I’ve also personally spoken to large Bitcoin services that rely on third party but want this feature.
It seems to me enabling this by default feels like the better direction, IMHO.

@gmaxwell
Copy link
Contributor

I think enabling it by default probably makes sense, that particular sub comment was more about having a ready way to choose not to use it when transacting.

As far as the initial defaults I think there I meant that that it shouldn't target too close to the bottom of the mempool as even if that currently gets high success that may not be true after wide deployment. Though on reflection I'm not sure that thats true as this only lowers feerates, so other people using this should only increase your odds of success while using it.

@ismaelsadeeq ismaelsadeeq force-pushed the 12-2025-fee-estimation-improvement branch from de42f89 to 9e2070f Compare January 11, 2026 17:36
@DrahtBot
Copy link
Contributor

🚧 At least one of the CI tasks failed.
Task 32 bit ARM: https://github.com/bitcoin/bitcoin/actions/runs/20899189424/job/60042443886
LLM reason (✨ experimental): Compilation failed due to a dangling reference error in mempool_estimator.cpp, treated as an error by -Werror.

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

ismaelsadeeq and others added 23 commits January 12, 2026 09:57
i  - FeeRateEstimator abstract class is the base class of fee rate estimators
     Derived classes must provide concrete implementation
     of the virtual methods.

ii - FeeRateEstimatorResult struct represent the response returned by
     a fee rate estimator.

iii - FeeRateEstimatorType will be used to identify fee estimators.
      Each time a new fee estimator is added, a corresponding
      enum value should be added.

Co-authored-by: willcl-ark <[email protected]>
- This commit updates CBlockPolicyEstimator class to implement
  the FeeRateEstimator interface.

Co-authored-by: willcl-ark <[email protected]>
- Introduce FeeRateEstimatorManager, a module for managing
  multiple fee rate estimators. This manager allows registration
  of multiple FeeRateEstimator instances, enabling having more fee rate
  estimation strategies to be used in the bitcoin core node and wallet.

Co-authored-by: willcl-ark <[email protected]>
- Add GetFeeRateEstimator(), a templated method that returns a specific
  estimator instance by type.
- Initialize FeeEstimatorManager in node instead of BlockPolicyEstimator.
- Forward fee rate estimation requests through FeeEstimatorManager.
- Update Chain interface to return FeeRateEstimatorResult, removing the out parameter requirement.
- Retain selected BlockPolicyEstimator methods for test-only estimaterawfee RPC compatibility.
- Given a vector of chunk feerates in the order they were added
  to the block, CalculatePercentiles function will return the 25th,
  50th, 75th and 95th percentile feerates.

- This commit also add a unit test for the CalculatePercentile function.
- The mempool fee rate estimator uses the unconfirmed transactions in the mempool
  to generate a fee rate that a package should have for it to confirm as soon as possible.

Co-authored-by: willcl-ark <[email protected]>
- Only refresh the cached fee rate estimate if it is older
      than 7 seconds.

- This avoids generating block templates too often.

Co-authored-by: willcl-ark <[email protected]>
- This ensures that the users mempool has seen at least 75% of the
  last 6 blocks transactions.
- This commit also enable estimatesmartfee to return
  the stats when verbosity level is greater than 1.
@ismaelsadeeq ismaelsadeeq force-pushed the 12-2025-fee-estimation-improvement branch from 9e2070f to 76b36fe Compare January 12, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improving fee estimation accuracy

4 participants