Fee Estimation via Fee rate Forecasters

ismaelsadeeq commented at 11:44 am on May 23, 2024: member

This PR is the complete implementation of #30392 it aim to improve Bitcoin Core Fee Estimation by:

Reducing Overestimation: Address and mitigate the overestimation issues present in the current BlockPolicyEstimator. This issue has been documented and acknowledged by various sources:
- See Delving bitcoin posts
- Rene-pickhard Research
Mempool Awareness: Enable the fee estimator to be aware of the mempool state, allowing it to react faster and more accurately to rapidly changing fee market conditions, when targeting very short timeframes.
Empowering Node Users: Allow node users to be self-sovereign by using their node’s estimates, reducing reliance on third-party fee estimations.
Simplifying Strategy Integration: Simplify the process of adding new fee estimation strategies in the future.

The detailed design document for this PR can be found here.

Please note that the design is subject to change as we refine the approach.

This is a collaborative effort with @willcl-ark and incorporates insights from other contributors.

DrahtBot commented at 11:44 am on May 23, 2024: contributor

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/30157.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	murchandamus, vasild

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32523 (wallet: Remove watchonly behavior and isminetypes by achow101)
#32463 (test: fix an incorrect feature_fee_estimation.py subtest by ismaelsadeeq)
#32395 (fees: rpc: estimatesmartfee now returns a fee rate estimate during low network activity by ismaelsadeeq)
#31664 (Fees: add Fee rate Forecaster Manager by ismaelsadeeq)
#31382 (kernel: Flush in ChainstateManager destructor by TheCharlatan)
#30079 (Fee Estimation: Ignore all transactions that are CPFP’d by ismaelsadeeq)
#28690 (build: Introduce internal kernel library by TheCharlatan)

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 typos and grammar issues:

“at least %d blocks needs to be processed after startup” → “need” [plural subject “blocks” requires “need”]
“the forecaster that provide the fee rate.” → “provides” [singular subject “forecaster” requires “provides”]

drahtbot_id_4_m

DrahtBot added the label CI failed on May 23, 2024

DrahtBot commented at 3:18 pm on May 23, 2024: contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is 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.

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

Debug: https://github.com/bitcoin/bitcoin/runs/25327260112

luke-jr commented at 5:20 pm on May 23, 2024: member

Make the fee estimator aware of the state of the mempool, allowing it to respond to changing conditions immediately.

The state of the node’s mempool may not accurately reflect the state of others’ mempools, and not even its own mempool when the block is found in the future. It isn’t a good single source of information. Perhaps it is a good idea to use it as a secondary source, but probably it should only ever adjust fee estimations upward, not down.

willcl-ark commented at 8:41 am on May 24, 2024: member

The state of the node’s mempool may not accurately reflect the state of others’ mempools, and not even its own mempool when the block is found in the future. It isn’t a good single source of information.

Correct. The rationale behind this set of changes can be summed up briefly as follows:

Add a new standalone modular fee estimation manager to which (many) Forcasters can be trivially added or removed (vs modifying BlockPolicyEstimator).
Provide two forcaster implementations which do not exist today ( 1. mempool-based and 2. previous-6-blocks-seen-in-mempool) to demonstrate functionality.

In the present, there is a strong tendency for users of Bitcoin Core to consult external fee estimation services when they want timely (next 1 or 2 blocks) confirmation of transactions, whilst also not overpaying. Examples of these include mempool.space, whatthefee.io, Samourai’s nextblock.is (now down), johoe, blockchair etc., with more popping up every month.

We also analysed/estimated Bitcoin Core users not using in-built estimation in a post here.

In our opinion having users feel the need to use external fee estimation services that they could equally have served to them by their own node, feels sub-optimal.

In addition to this, when users do use the current Bitcoin Core fee estimator, there are often times when they end up overpaying, see delving bitcoin post Mempool based fee estimation and various issues over the years e.g. #30009 . This is avoidable.

Work from a student of @renepickhardt link demonstrated something we also measured independently – that bitcoin core’s current estimates are often overpaying significantly following fee spikes. This effect can be directly mitigated by using a mempool-based estimation.

Perhaps it is a good idea to use it as a secondary source, but probably it should only ever adjust fee estimations upward, not down.

In this changeset we take the approach of using the lowest result from all “confident” Forcasters. The rationale is that we expect users wanting fast confirmation to have RBF enabled, allowing them to bump fees if we still undershoot.

Comments from @harding link explained that it may be possible for miners to artificially increase a strictly-mempool-based fee-rate. By taking the lower (confident) result from n Forcasters, we attempt to protect against this attack (and others like it), at the potential cost of having to RBF.

We do plan to add additional sanity checks to the mempool-based Forcaster as described in #27995, but these are not yet implemented. In any case, even without these additional checks we have been seeing much-improved short time-scale estimations from Bitcoin Core.

ismaelsadeeq force-pushed on May 29, 2024

DrahtBot removed the label CI failed on May 29, 2024

DrahtBot added the label Needs rebase on Jun 11, 2024

ismaelsadeeq force-pushed on Jul 3, 2024

DrahtBot removed the label Needs rebase on Jul 3, 2024

ismaelsadeeq force-pushed on Jul 3, 2024

DrahtBot added the label CI failed on Jul 3, 2024

DrahtBot commented at 5:02 pm on July 3, 2024: contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is 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.

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

Debug: https://github.com/bitcoin/bitcoin/runs/27003108803

DrahtBot removed the label CI failed on Jul 3, 2024

ismaelsadeeq force-pushed on Jul 8, 2024

DrahtBot added the label Needs rebase on Jul 8, 2024

ismaelsadeeq force-pushed on Jul 8, 2024

DrahtBot removed the label Needs rebase on Jul 8, 2024

DrahtBot added the label Needs rebase on Jul 18, 2024

ismaelsadeeq force-pushed on Jul 24, 2024

DrahtBot commented at 12:14 pm on July 24, 2024: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/27856068740

Make sure to run all tests locally, according to the documentation.

The failure may 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.

DrahtBot added the label CI failed on Jul 24, 2024

willcl-ark added the label Needs Conceptual Review on Jul 24, 2024

DrahtBot removed the label Needs rebase on Jul 24, 2024

ismaelsadeeq force-pushed on Jul 24, 2024

DrahtBot removed the label CI failed on Jul 24, 2024

ismaelsadeeq force-pushed on Aug 13, 2024

DrahtBot added the label CI failed on Aug 13, 2024

ismaelsadeeq force-pushed on Aug 13, 2024

DrahtBot removed the label CI failed on Aug 13, 2024

hebasto added the label Needs CMake port on Aug 16, 2024

DrahtBot added the label Needs rebase on Aug 21, 2024

glozow added the label TX fees and policy on Aug 21, 2024

maflcko removed the label Needs CMake port on Aug 29, 2024

ismaelsadeeq force-pushed on Sep 3, 2024

ismaelsadeeq commented at 9:43 am on September 3, 2024: member

Rebased and added newly introduced files to cmakelist file instead makefile

DrahtBot removed the label Needs rebase on Sep 3, 2024

DrahtBot added the label Needs rebase on Sep 20, 2024

ismaelsadeeq force-pushed on Oct 14, 2024

DrahtBot removed the label Needs rebase on Oct 14, 2024

DrahtBot added the label CI failed on Oct 14, 2024

DrahtBot commented at 5:03 pm on October 14, 2024: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/31509135850

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.

murchandamus commented at 5:57 pm on October 21, 2024: contributor

Concept ACK! Thank you for working on this.

vasild commented at 4:28 am on November 6, 2024: contributor

Concept ACK

Allow node users to be self-sovereign by using their node’s estimates, reducing reliance on third-party fee estimations.

having users feel the need to use external fee estimation services that they could equally have served to them by their own node, feels sub-optimal

Indeed, I do that :face_with_head_bandage:

ismaelsadeeq force-pushed on Nov 11, 2024

DrahtBot removed the label CI failed on Nov 11, 2024

ismaelsadeeq force-pushed on Nov 14, 2024

DrahtBot added the label CI failed on Nov 14, 2024

DrahtBot commented at 8:52 pm on November 14, 2024: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/33010015546

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 force-pushed on Nov 14, 2024

ismaelsadeeq commented at 9:52 pm on November 14, 2024: member

Thanks for your conceptual review, @murchandamus and @vasild. There’s also another review by @remyers in the tracking issue: #30392 (comment).

I have updated this PR based on feedback from #30391 and in-person conversations:

I am now using the newly introduced FeeFrac datatype, which does not lose precision like CFeeRate.
Updated the CalculatePercentile function to return a monotonically decreasing estimate for high and low priority. The previous approach took the fee rate of the package at the exact percentile, which could be high due to the ancestors of the percentile package included previously in a sibling package.

Relevant commit: 2ac86a8fd7fe108078511ef2f6ad77e315591a2b. I’ve also added a test for this.

Added a functional test for the estimatefee RPC behavior (commit ac8f2caf050e73b038a8e03a51dfe69e1ea7aa19).
Removed the tracing commit after merging #26593, as it changes the tracing structure, which I haven’t reviewed yet. The tracing commit was relevant for collecting node’s estimation data for analysis, it can be added later on.

This PR has now received a conceptual review from a few contributors, so we can proceed to code and approach review.

DrahtBot removed the label CI failed on Nov 14, 2024

DrahtBot added the label Needs rebase on Nov 20, 2024

ismaelsadeeq force-pushed on Nov 25, 2024

DrahtBot removed the label Needs rebase on Nov 25, 2024

DrahtBot added the label Needs rebase on Dec 17, 2024

ismaelsadeeq force-pushed on Jan 2, 2025

DrahtBot removed the label Needs rebase on Jan 2, 2025

DrahtBot added the label Needs rebase on Jan 6, 2025

glozow referenced this in commit 66aa6a47bd on Jan 8, 2025

ismaelsadeeq force-pushed on Jan 15, 2025

DrahtBot added the label CI failed on Jan 15, 2025

ismaelsadeeq force-pushed on Jan 15, 2025

DrahtBot removed the label CI failed on Jan 15, 2025

DrahtBot removed the label Needs rebase on Jan 16, 2025

DrahtBot added the label Needs rebase on Jan 22, 2025

ismaelsadeeq force-pushed on Jan 22, 2025

DrahtBot removed the label Needs rebase on Jan 22, 2025

DrahtBot added the label Needs rebase on Mar 11, 2025

in src/rpc/fees.cpp:103 in e8c4e88c71 outdated

 95@@ -94,6 +96,76 @@ static RPCHelpMan estimatesmartfee()
 96     };
 97 }
 98 
 99+static RPCHelpMan estimatefee()
100+{
101+    return RPCHelpMan{
102+        "estimatefee",
103+        "\nEstimates the approximate fee per kilobyte needed for a transaction to begin\n"

yancyribbens commented at 9:29 pm on March 11, 2025:

This is confusing I think. It’s titled estimatefee however a fee is usually a bitcoin amount. A feerate however is a bitcoin amount per size.

yancyribbens commented at 9:30 pm on March 11, 2025:

should this be “estimatefeerate”?

ismaelsadeeq commented at 11:43 am on March 12, 2025:

I will update this here #31664

ismaelsadeeq commented at 2:31 pm on April 13, 2025:

The new RPC is gone; I’ve integrated this new feature into estimatesmartfee and maintained backward compatibility for users who want only block policy, using a flag.

in src/test/mempoolforecaster_tests.cpp:51 in e8c4e88c71 outdated

46+        // Test when targetBlocks > MEMPOOL_FORECAST_MAX_TARGET
47+        const auto fee_estimate = mempool_fee_estimator->EstimateFee(conf_target);
48+        BOOST_CHECK(fee_estimate.Empty());
49+        BOOST_CHECK(*fee_estimate.GetError() == strprintf("Confirmation target %s exceeds the maximum limit of %s. mempool conditions might change, "
50+                                                          "making forecasts above %s block may be unreliable",
51+                                                          MEMPOOL_FORECAST_MAX_TARGET + 1, MEMPOOL_FORECAST_MAX_TARGET, MEMPOOL_FORECAST_MAX_TARGET));

yancyribbens commented at 9:33 pm on March 11, 2025:

shouldn’t the first positional argument be conf_target ?

ismaelsadeeq commented at 11:45 am on March 12, 2025:

No, conf_target is a struct can not be passed to strprintf

yancyribbens commented at 3:16 pm on March 12, 2025:

Ok, it’s just that you are using MEMPOOL_FORECAST_MAX_TARGET + 1 as the positional argument for Confirmation target %s which doesn’t seem right, but maybe I’m mistaken.

in src/test/mempoolforecaster_tests.cpp:64 in e8c4e88c71 outdated

61+
62+    conf_target.value = MEMPOOL_FORECAST_MAX_TARGET;
63+    // Test when there are not enough mempool transactions to get an accurate estimate
64+    {
65+        std::vector<CTransactionRef> highfee_txs;
66+        // Add transactions with high_fee fee until mempool transactions weight is more than 25th percent of DEFAULT_BLOCK_MAX_WEIGHT

yancyribbens commented at 9:34 pm on March 11, 2025:

0        // Add transactions with high_fee until mempool transactions weight is more than 25th percent of DEFAULT_BLOCK_MAX_WEIGHT

yancyribbens commented at 9:36 pm on March 11, 2025:

than 25th percent of DEFAULT_BLOCK_MAX_WEIGHT

Maybe others understand this, but I’m not sure what 25th percent of … means. Is that the same as 25% of ..?

ismaelsadeeq commented at 11:48 am on March 12, 2025:

It is 0.25 * DEFAULT_BLOCK_MAX_WEIGHT see https://delvingbitcoin.org/t/mempool-based-fee-estimation-on-bitcoin-core/703/2 where the term was used.

yancyribbens commented at 3:20 pm on March 12, 2025:

Oh, you mean to say 25th percentile of DEFAULT_BLOCK_MAX_WEIGHT

ismaelsadeeq force-pushed on Apr 13, 2025

DrahtBot added the label CI failed on Apr 13, 2025

DrahtBot commented at 1:18 pm on April 13, 2025: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/40463061038

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.

DrahtBot removed the label Needs rebase on Apr 13, 2025

ismaelsadeeq force-pushed on Apr 13, 2025

ismaelsadeeq commented at 2:34 pm on April 13, 2025: member

Rebased

DrahtBot removed the label CI failed on Apr 13, 2025

Babam1111qqqqqq11qw commented at 3:13 pm on April 13, 2025: none

سلام…من یه مدت درگیر مشکلات شخصی بودم…به هیچ چیزی دسترسی نداشتم…چند نفر بجای من فعال بود…چیززیادی از دست دادم.؟؟لطفا کمک کنید..بتونم مشکل رو حل کنم.. tabasom12

در تاریخ یکشنبه ۱۳ آوریل ۲۰۲۵،‏ ۱۹:۰۴ Abubakar Sadiq Ismail < @.***> نوشت:

Rebased

— Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/30157#issuecomment-2799977043, or unsubscribe https://github.com/notifications/unsubscribe-auth/BJOKXGB3E2EI4XK5HP3OODD2ZJYYXAVCNFSM6AAAAABIFPE2XKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOOJZHE3TOMBUGM . You are receiving this because you are subscribed to this thread.Message ID: @.***> ismaelsadeeq left a comment (bitcoin/bitcoin#30157) https://github.com/bitcoin/bitcoin/pull/30157#issuecomment-2799977043

Rebased

— Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/30157#issuecomment-2799977043, or unsubscribe https://github.com/notifications/unsubscribe-auth/BJOKXGB3E2EI4XK5HP3OODD2ZJYYXAVCNFSM6AAAAABIFPE2XKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOOJZHE3TOMBUGM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Ferrydh commented at 3:17 pm on April 13, 2025: none

这是来自QQ邮箱的假期自动回复邮件。您好，我最近正在休假中，无法亲自回复您的邮件。我将在假期结束后，尽快给您回复。

DrahtBot added the label Needs rebase on Apr 14, 2025

ismaelsadeeq force-pushed on Apr 15, 2025

DrahtBot removed the label Needs rebase on Apr 15, 2025

ismaelsadeeq force-pushed on Apr 25, 2025

ismaelsadeeq force-pushed on Apr 27, 2025

DrahtBot added the label Needs rebase on May 13, 2025

ismaelsadeeq force-pushed on May 14, 2025

DrahtBot removed the label Needs rebase on May 14, 2025

DrahtBot added the label Needs rebase on May 20, 2025

ismaelsadeeq force-pushed on May 20, 2025

DrahtBot removed the label Needs rebase on May 20, 2025

ismaelsadeeq force-pushed on May 21, 2025

DrahtBot added the label CI failed on May 21, 2025

DrahtBot commented at 5:48 pm on May 21, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/42632378606 LLM reason (✨ experimental): The CI failure is due to lint errors, specifically the use of assert instead of CHECK_NONFATAL or NONFATAL_UNREACHABLE in RPC code.

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 force-pushed on May 21, 2025

DrahtBot removed the label CI failed on May 22, 2025

DrahtBot added the label CI failed on Jun 10, 2025

ismaelsadeeq force-pushed on Jun 12, 2025

fees: refactor: rename policy_fee_tests.cpp to feerounder_tests.cpp

- Also remame the test suite name to match the new name.

e1f77bf61b

fees: refactor: rename fees to block_policy_estimator

- Also move it to policy/fees and update the includes

5e899b8d5d

fees: rename fees_args to block_policy_estimator_args

- Also move them to policy/fees/ and update includes
- Note: the block_policy_estimator_args.h include in block_policy_estimator_args.cpp was done manually.

ce62fe553a

fees: return current block height in `estimateSmartFee` d700d1c765

fees: add forecaster structures

i  - Forecaster abstract class is the base class of fee rate forecasters.
     Derived classes must provide concrete implementation
     of the virtual methods.

ii - ForecastResult struct represent the response returned by
     a fee rate forecaster.

iii - ForecastType will be used to identify forecasters.
      Each time a new forecaster is added, a corresponding
      enum value should be added to ForecastType.

Co-authored-by: willcl-ark <will@256k1.dev>

e54515445d

fees: add ForecasterMan class

- Its a module for managing and utilising multiple
  fee rate forecasters to provide fee rate forecast.

- The ForecasterManager class allows for the registration of
  multiple fee rate forecasters.

Co-authored-by: willcl-ark <will@256k1.dev>

d497bee4f5

fees: make block_policy_estimator a forecaster 380a5b9fb8

fees: add block policy estimator to forecaster manager

- This changes `CBlockPolicyEstimator` to a shared pointer
  this gives us three advantages.
   - Registering to validation interface using shared pointer
   - Scheduling block policy estimator flushes using shared pointer
   - Registering block policy estimator to forecaster_man

cd02ca612a

fees: add `forecastTypeToString` method

- This method converts a ForecastType enum to its
  string representation.

fd4b13981b

fees: add `CalculatePercentiles` function

- The CalculatePercentiles function, given
  a vector of feerates in the order they were added
  to the block, will return the 25th, 50th, 75th,
  and 95th percentile feerates.

- Also add a unit test for this function.

b78587a444

fees: add `MemPoolForecaster` class

- The mempool forecaster 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 <will@256k1.dev>

a4a06ee916

test: add mempool forecaster unit test cef8091d33

fees: cache `MemPoolPolicyEstimator` forecasts

- Provide new forecast only when the time delta from previous
  forecast is older than 30 seconds.

- This caching helps avoid the high cost of frequently generating block templates.

Co-authored-by: willcl-ark <will@256k1.dev>

c9c8538f61

fees: add `ForecastFeeRateFromForecasters` method

- Polls all registered forecasters and selects the lowest fee rate.

c6d041f2e6

===== End of FeeRateForecasterMan commits ===== cfa3f607dd

rpc: update `estimatefeesmartfee` to use forecaster_man

- Given a confirmation target, we use fee estimator module that call all
available fee estimator forcasters and return the lowest fee rate that if
a transaction use will likely confirm in a given confirmation target.

- This also provides backward compatibility

Co-authored-by: willcl-ark <will@256k1.dev>

8776d1a19a

test: add `estimatesmartfee` new functionality 4de8a70def

===== End of RPC commits ===== a01e84a655

fees: introduce locks to forecaster manager and sanity checks

- Also functional test

581f1c81a2

==== End Of Sanity Check ==== 10faac411c

ismaelsadeeq force-pushed on Jun 15, 2025

willcl-ark commented at 8:42 am on June 17, 2025: member

I restarted my relatively long-running node after building this PR and notice that now I get the following when trying to get any estimate:

 0src/core/bitcoin on  pr-30157 [$?] via △ v3.31.6 via 🐍 v3.13.3 via ❄️  impure (nix-shell-env)
 1❯ /home/will/src/core/bitcoin/build/bin/bitcoin-cli estimatesmartfee 1
 2{
 3  "blocks": 0,
 4  "errors": [
 5    "Insufficient data (at least 6 blocks needs to be processed after startup)"
 6  ]
 7}
 8
 9src/core/bitcoin on  pr-30157 [$?] via △ v3.31.6 via 🐍 v3.13.3 via ❄️  impure (nix-shell-env)
10❯ /home/will/src/core/bitcoin/build/bin/bitcoin-cli estimatesmartfee 3
11{
12  "blocks": 0,
13  "errors": [
14    "Insufficient data (at least 6 blocks needs to be processed after startup)"
15  ]
16}

This is not the case when I restart v29.0:

0src/core/bitcoin on  pr-30157 [$?] via △ v3.31.6 via 🐍 v3.13.3 via ❄️  impure (nix-shell-env)
1❯ bitcoind
2Bitcoin Core starting
3
4src/core/bitcoin on  pr-30157 [$?] via △ v3.31.6 via 🐍 v3.13.3 via ❄️  impure (nix-shell-env)
5❯ /home/will/src/core/bitcoin/build/bin/bitcoin-cli estimatesmartfee 2
6{
7  "feerate": 0.00005081,
8  "blocks": 2
9}

It’s able to return a fee estimate to me immediately, as it has saved estimation data from the previous run. I don’t think we should break this behaviour here; waiting ~ an hour to get a new estimate after a restart is not a pleasant UX.

At the very least we should not nerf blockpolicy estimations as part of this change. Although, I think thought that we should be smart enough to know if we have a ~ up-to-date mempool and use that data if we do, to return estimates via the mempool-based estimator. If we need, perhaps we can return a warning about the age of the mempool when returning e.g. a next-block estimate, although with RBF prevalance now I’m not sure even that is necessary?

ismaelsadeeq commented at 9:54 am on June 17, 2025: member

I restarted my relatively long-running node after building this PR and notice that now I get the following when trying to get any estimate

Yep, the node needs to see six blocks mined before making an estimate. Were you able to see an estimate after six blocks were mined?

It’s able to return a fee estimate to me immediately, as it has saved estimation data from the previous run. I don’t think we should break this behaviour here; waiting ~ an hour to get a new estimate after a restart is not a pleasant UX.

At the very least we should not nerf blockpolicy estimations as part of this change. Although, I think thought that we should be smart enough to know if we have a ~ up-to-date mempool and use that data if we do, to return estimates via the mempool-based estimator. If we need, perhaps we can return a warning about the age of the mempool when returning e.g. a next-block estimate, although with RBF prevalance now I’m not sure even that is necessary?

There is a change in behavior. Before restarting, it was solely using the block policy. Now, it uses both the block policy and the mempool. The forecaster manager needs to see some blocks mined and determine whether the mempool policy is sane. It should work fine after six blocks are mined (I agree that six blocks is a bit extreme; note: it just a conservative arbitrary value to be sure).

This methodology aligns with the current status quo of not returning anything when we don’t have enough data. Although, I think we should return something which is why I modified block policy to do so in #32395. It’s true that this isn’t a pleasant UX, and I think we might see more complaints if this is rolled out as-is.

So perhaps we should return the block policy estimate if it’s available in the meantime and reduce NUMBER_OF_BLOCKS?

willcl-ark commented at 9:58 am on June 17, 2025: member

Yep, the node needs to see six blocks mined before making an estimate. Were you able to see an estimate after six blocks were mined?

I’m still waiting 😄 but I assume that I will be able to eventually…

When we have both fee_estimates.dat and mempool.dat saving all the relevant data I don’t think we should be introducing a 6 block wait to get fee estimates. My data was ~ 10 seconds out of date after a restart, and now I must wait an hour.

Fee Estimation via Fee rate Forecasters #30157

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)