fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates

ismaelsadeeq commented at 3:15 pm on August 15, 2025: member

This is a simple PR that updates the block policy estimator’s MIN_BUCKET_FEERATE constant to be identical to the policy DEFAULT_MIN_RELAY_TX_FEE. It also enables the fee estimator to record transactions with a fee rate up to the DEFAULT_MIN_RELAY_TX_FEE in its stats, which effectively allows the estimator to return sub-1 sat/vB fee rate estimates.

The change is correct because the estimator creates buckets of fee rates from MIN_BUCKET_FEERATE, MIN_BUCKET_FEERATE + FEE_SPACING, MIN_BUCKET_FEERATE + 2 * FEE_SPACING, … up to MAX_BUCKET_FEERATE.

This means it will record sub-1 sat/vB fee rates in the buckets and may return them as a fee rate estimate when that bucket is the lowest one with sufficient transactions for a given target.

Tying MIN_BUCKET_FEERATE to DEFAULT_MIN_RELAY_TX_FEE also makes it dynamic, so we don’t need to update the fee estimator when the minrelaytxfee default is adjusted in the future.

For the mempool fee rate estimator in #31664, no update is needed since it uses the block assembler and because the node can see sub-1 sat/vB transactions, it can generate block templates with those and therefore return a fee rate estimate accordingly.

DrahtBot commented at 3:15 pm on August 15, 2025: 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/33199.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	polespinasa, kevkevinpal

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:

#33214 (rpc: require integer verbosity; remove boolean ‘verbose’ by fqlx)
#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)
#30157 (Fee Estimation via Fee rate Forecasters by ismaelsadeeq)
#30079 (Fee Estimation: Ignore all transactions that are CPFP’d by ismaelsadeeq)

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:

- with sufficient data, it’s average value will be returned as the fee rate estimate. -> its average value will be returned as the fee rate estimate. [“it’s” (contraction of “it is”) is incorrect here; the possessive “its” is required.]

   self.log.info("Test that estimatesmartfee return sub 1s/vb fee rate estimate") -> self.log.info("Test that estimatesmartfee returns a sub 1s/vb fee rate estimate") [subject-verb agreement and article: "estimatesmartfee return" should be "estimatesmartfee returns" and include "a" for clarity.]

drahtbot_id_5_m

ismaelsadeeq force-pushed on Aug 15, 2025

in test/functional/feature_fee_estimation.py:443 in 413c81b134 outdated

431+            self.broadcast_and_mine(self.nodes[1], self.nodes[2], low_feerate, TXS_COUNT)
432         # conservative mode will consider longer time horizons while economical mode does not
433         # Check the fee estimates for both modes after mining low fee transactions.
434         check_fee_estimates_btw_modes(self.nodes[0], high_feerate, low_feerate)
435 
436+    def test_sub_1s_per_vb_estimates(self):

polespinasa commented at 5:39 pm on August 19, 2025:

I was wondering if the new test could be avoided by defining low_feerate as MINIMUM_RELAY_FEE_RATE in the previous test test_estimation_modes.

ismaelsadeeq commented at 6:03 pm on August 19, 2025:

The test_sub_1s_per_vb_estimates is not even necessary hence can be removed because we have a general test in sanity_check_estimates_range.

I added test_sub_1s_per_vb_estimates to convince reviewers and myself that it works.

However that said, I don’t think its okay to bundle test together like this, but curious to know your opinion on why we would want to avoid adding the test.

polespinasa commented at 6:10 pm on August 19, 2025:

Less code the better :)

It’s not a new or modified feature to be tested, it’s just a value that has been lowered.

kevkevinpal commented at 11:54 am on August 26, 2025:

Just an fyi, this test passes even without the changes in src/policy/fees.h I also agree with @polespinasa that if another test already covers this, then I’d rather not have redundant tests added

ismaelsadeeq commented at 12:13 pm on September 1, 2025:

This is a nice catch. The reason is that all transactions the fee estimator sees with a fee rate below 1 sat/vB are added to the 1–1.1 sat/vB fee rate bucket. When that bucket is the lowest with sufficient data, the average is taken, which allows it to pass.

I modified the change to be more accurate by mixing in some 1 sat/vB transactions. This way, it will fail on master and also serve as a test case for that edge case.

Hence I think we require this test I will resolve #33199 (review) cc @polespinasa

polespinasa commented at 5:42 pm on August 19, 2025: contributor

concept ACK

This change makes much sense to me, if the min relay fee is lowered the feerate tracking should also take that into account.

in src/policy/fees.h:186 in 413c81b134 outdated

180@@ -180,13 +181,10 @@ class CBlockPolicyEstimator : public CValidationInterface
181     static constexpr double SUFFICIENT_TXS_SHORT = 0.5;
182 
183     /** Minimum and Maximum values for tracking feerates
184-     * The MIN_BUCKET_FEERATE should just be set to the lowest reasonable feerate we
185-     * might ever want to track.  Historically this has been 1000 since it was
186-     * inheriting DEFAULT_MIN_RELAY_TX_FEE and changing it is disruptive as it
187-     * invalidates old estimates files. So leave it at 1000 unless it becomes

kevkevinpal commented at 11:58 am on August 26, 2025:

I am not sure if we need a release note, since according to this note, it will invalidate old estimates files? Maybe someone else can chime in on this

ismaelsadeeq commented at 12:05 pm on September 1, 2025:

Done

kevkevinpal commented at 11:59 am on August 26, 2025: contributor

Concept ACK 413c81b

It makes sense to update MIN_BUCKET_FEERATE with the recent changes to be sub 1 sat/vbyte

I do think the additional test is a bit redundant but I am okay in adding if others think it makes sense

maflcko commented at 10:52 am on August 28, 2025: member

This picks up #13990? If yes, it may be good to explain the differences.

maflcko added the label TX fees and policy on Aug 28, 2025

fees: make `MIN_BUCKET_FEERATE` identical to `DEFAULT_MIN_RELAY_TX_FEE` 78610ca567

ismaelsadeeq force-pushed on Sep 1, 2025

fees: bump fees file version 5480deac2e

test: ensure fee estimator provide fee rate estimate < 1 s/vb

- This commit also refactors the test the better track confirmed
  utxos.

5bf472065d

doc: add release notes 0eb02ea1a3

ismaelsadeeq force-pushed on Sep 1, 2025

ismaelsadeeq commented at 1:28 pm on September 1, 2025: member

This picks up #13990? If yes, it may be good to explain the differences.

This is different from #13990 in approach:

It omits the consistency check and avoids extending the fee rate buckets when the min bucket fee rate changes. Instead, it builds on #29702 by simply bumping the fees file version, which ensures that estimates saved in previous files are not read. As such after a restart, the estimator starts afresh.

in doc/release-notes-33199.md:7 in 0eb02ea1a3

0@@ -0,0 +1,9 @@
1+Fee Estimation
2+========================
3+
4+- The Bitcoin Core fee estimator minimum fee rate bucket was updated from **1 sat/vB** to **0.1 sat/vB**,
5+  which matches the node’s default `minrelayfee`.
6+  This means that for a given confirmation target, if a sub-1 sat/vB fee rate bucket is the minimum tracked
7+  with sufficient data, it's average value will be returned as the fee rate estimate.

maflcko commented at 2:17 pm on September 1, 2025:

with sufficient data, it's average value will be returned as the fee rate estimate. -> its average value will be returned as the fee rate estimate. ["it's" (contraction of "it is") is incorrect here; the possessive "its" is required.]

in test/functional/feature_fee_estimation.py:499 in 0eb02ea1a3

495         self.clear_estimates()
496         self.log.info("Test estimatesmartfee modes")
497         self.test_estimation_modes()
498 
499+        self.clear_estimates()
500+        self.log.info("Test that estimatesmartfee return sub 1s/vb fee rate estimate")

maflcko commented at 2:17 pm on September 1, 2025:

 self.log.info("Test that estimatesmartfee return sub 1s/vb fee rate estimate") -> self.log.info("Test that estimatesmartfee returns a sub 1s/vb fee rate estimate") [subject-verb agreement and article: "estimatesmartfee return" should be "estimatesmartfee returns" and include "a" for clarity.]

fees: enable CBlockPolicyEstimator return sub 1 sat/vb fee rate estimates #33199

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates #33199