fees: refactor: separate feerate format from fee estimate mode

ismaelsadeeq commented at 6:21 pm on February 10, 2026: member

Motivation

Part of #34075

The FeeEstimateMode enum was responsible for both selecting the fee estimation algorithm and specifying the fee rate’ format.

Changes in this PR:

The FeeEstimateMode enum (UNSET, ECONOMICAL, CONSERVATIVE) is moved to a new util/fees.h header.
A new FeeRateFormat enum (BTC_KVB, SAT_VB) is introduced in policy/feerate.h for feerate formatting.
The CFeeRate::ToString() method is updated to use FeeRateFormat.
All relevant function calls have been updated to use the new FeeRateFormat enum for formatting and FeeEstimateMode for fee estimation mode.

This refactoring separates these unrelated responsibilities to improve code clarity.

DrahtBot added the label TX fees and policy on Feb 10, 2026

DrahtBot commented at 6:22 pm on February 10, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	l0rinc, furszy, musaHaruna, willcl-ark

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34075 (fees: Introduce Mempool Based Fee Estimation to reduce overestimation 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.

ismaelsadeeq commented at 6:24 pm on February 10, 2026: member

cc @l0rinc, @Eunovo, as discussed IRL.

I also split it into two commits as you asked.

in src/common/messages.cpp:1 in ea62e21ec3 outdated

l0rinc commented at 9:04 pm on February 10, 2026:

nit: consider a slightly more descriptive commit message since a few things have happened here:

0fees: split feerate format from estimate mode
1
2Introduce `FeeRateFormat` in `policy/feerate.h` and change `CFeeRate::ToString()` to use it for `BTC/kvB` vs `sat/vB` output formatting.
3
4Keep `FeeEstimateMode` focused on fee estimation behavior by removing formatting values from `util/fees.h`.
5
6Update all formatting call sites and tests to pass `FeeRateFormat` explicitly, separating fee-rate display concerns from fee-estimation mode selection.

in src/policy/feerate.cpp:9 in eefd9d5427

5@@ -6,6 +6,7 @@
6 #include <consensus/amount.h>
7 #include <policy/feerate.h>
8 #include <tinyformat.h>
9+#include <util/fees.h>

l0rinc commented at 9:05 pm on February 10, 2026:

eefd9d5427aa12ab7dbd0501f3e063b212523bbd I don’t think we need this in the first commit

in src/policy/feerate.cpp:30 in ea62e21ec3

26@@ -27,11 +27,11 @@ CAmount CFeeRate::GetFee(int32_t virtual_bytes) const
27     return nFee;
28 }
29 
30-std::string CFeeRate::ToString(const FeeEstimateMode& fee_estimate_mode) const
31+std::string CFeeRate::ToString(const FeeRateFormat& feerate_format) const

l0rinc commented at 9:07 pm on February 10, 2026:

the enum name splits the word fee from format:

0std::string CFeeRate::ToString(const FeeRateFormat& fee_rate_format) const

or just

0std::string CFeeRate::ToString(const FeeRateFormat& format) const

in src/policy/feerate.cpp:35 in ea62e21ec3

33     const CAmount feerate_per_kvb = GetFeePerK();
34-    switch (fee_estimate_mode) {
35-    case FeeEstimateMode::SAT_VB: return strprintf("%d.%03d %s/vB", feerate_per_kvb / 1000, feerate_per_kvb % 1000, CURRENCY_ATOM);
36+    switch (feerate_format) {
37+    case FeeRateFormat::SAT_VB: return strprintf("%d.%03d %s/vB", feerate_per_kvb / 1000, feerate_per_kvb % 1000, CURRENCY_ATOM);
38     default:                      return strprintf("%d.%08d %s/kvB", feerate_per_kvb / COIN, feerate_per_kvb % COIN, CURRENCY_UNIT);

l0rinc commented at 9:08 pm on February 10, 2026:

since we have a dedicated enum, we don’t need a default anymore, consider something like:

0std::string CFeeRate::ToString(const FeeRateFormat& format) const
1{
2    const CAmount feerate_per_kvb{GetFeePerK()};
3    switch (format) {
4    case FeeRateFormat::BTC_KVB: return strprintf("%d.%08d %s/kvB", feerate_per_kvb / COIN, feerate_per_kvb % COIN, CURRENCY_UNIT);
5    case FeeRateFormat::SAT_VB:  return strprintf("%d.%03d %s/vB", feerate_per_kvb / 1000, feerate_per_kvb % 1000, CURRENCY_ATOM);
6    } // no default case, so the compiler can warn about missing cases
7    assert(false);
8}

in src/wallet/rpc/spend.cpp:16 in ea62e21ec3

11@@ -12,6 +12,8 @@
12 #include <rpc/rawtransaction_util.h>
13 #include <rpc/util.h>
14 #include <script/script.h>
15+#include <univalue.h>
16+#include <util/fees.h>

l0rinc commented at 9:14 pm on February 10, 2026:

do we need the FeeEstimateMode in this file?

in src/wallet/spend.cpp:20 in ea62e21ec3

16@@ -19,6 +17,7 @@
17 #include <script/signingprovider.h>
18 #include <script/solver.h>
19 #include <util/check.h>
20+#include <util/fees.h>

l0rinc commented at 9:16 pm on February 10, 2026:

we don’t seem to need FeeEstimateMode here either after the changes

l0rinc approved

l0rinc commented at 9:20 pm on February 10, 2026: contributor

Change looks mostly good to me, I would prefer unifying the commit messages and removing the default case from CFeeRate::ToString and some imports that may not be needed after the split.

refactor: move-only: move `FeeEstimateMode` enum to `util/fees.h` 922ebf96ed

ismaelsadeeq force-pushed on Feb 11, 2026

ismaelsadeeq commented at 12:43 pm on February 11, 2026: member

Forced pushed from ea62e21ec3f7e733fa89a77cde4003545f426f2f to 501347649cf4f02e0762350fd54a09d1315e14d5 compare diff 5f426f2f..501347

I addressed review comments from @l0rinc, thanks. #34552 (review) Taken with a few modifications. #34552 (review) We don’t, removed. #34552 (review) Taken, but did not modify other occurrences in the method. Doing that is beyond the scope of this PR. should be done everywhere. #34552 (review) Taken. #34552 (review) We don’t, removed. #34552 (review) Indeed removed.

ismaelsadeeq force-pushed on Feb 11, 2026

DrahtBot added the label CI failed on Feb 11, 2026

DrahtBot commented at 12:49 pm on February 11, 2026: contributor

🚧 At least one of the CI tasks failed. Task Alpine (musl): https://github.com/bitcoin/bitcoin/actions/runs/21905411377/job/63244294391 LLM reason (✨ experimental): Compiler error: feerate.cpp::ToString lacks a return value, with -Werror treating this warning as an error.

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 Feb 11, 2026

DrahtBot removed the label CI failed on Feb 11, 2026

l0rinc approved

l0rinc commented at 2:15 pm on February 11, 2026: contributor

ACK 501347649cf4f02e0762350fd54a09d1315e14d5

in src/policy/feerate.cpp:29 in 501347649c

25@@ -26,11 +26,13 @@ CAmount CFeeRate::GetFee(int32_t virtual_bytes) const
26     return nFee;
27 }
28 
29-std::string CFeeRate::ToString(const FeeEstimateMode& fee_estimate_mode) const
30+std::string CFeeRate::ToString(const FeeRateFormat& fee_rate_format) const

furszy commented at 3:06 pm on February 11, 2026:

nano nit: usually, enums are passed by value.

l0rinc commented at 3:11 pm on February 11, 2026:

indeed, we should fix that while we’re here

ismaelsadeeq commented at 3:49 pm on February 11, 2026:

Done in latest push https://github.com/bitcoin/bitcoin/compare/501347649cf4f02e0762350fd54a09d1315e14d5..c1355493e2c26b613109bfac3dcd898b3acca75a

furszy commented at 3:07 pm on February 11, 2026: member

ACK 501347649cf4f02e0762350fd54a09d1315e14d5

refactor: fees: split fee rate format from fee estimate mode

 - Introduce a `FeeRateFormat` enum and change `CFeeRate::ToString()`
   to use it for `BTC/kvB` vs `sat/vB` output formatting.
 - Handle all enum values, hence remove default case in `CFeeRate::ToString()`
   and `assert(False)` when a `FeeRateFormat` value is not handled.
 - Keep `FeeEstimateMode` focused on fee estimation behavior by removing fee rate format
   values from `FeeEstimateMode`.
 - Update all formatting call sites and tests to pass `FeeRateFormat` explicitly, separating fee rate format
   from fee-estimation mode selection.

c1355493e2

ismaelsadeeq force-pushed on Feb 11, 2026

in src/util/fees.h:17 in 922ebf96ed

12+    CONSERVATIVE, //!< Force estimateSmartFee to use conservative estimates
13+    BTC_KVB,      //!< Use BTC/kvB fee rate unit
14+    SAT_VB,       //!< Use sat/vB fee rate unit
15+};
16+
17+#endif // BITCOIN_UTIL_FEES_H

furszy commented at 3:57 pm on February 11, 2026:

The missing blank line at the end of the file will likely cause the CI lint job to fail

l0rinc commented at 4:18 pm on February 11, 2026:

GitHub is probably drunk:

ismaelsadeeq commented at 4:18 pm on February 11, 2026:

I don’t think so, the C. I was green previously.

furszy commented at 4:24 pm on February 11, 2026:

I don’t think so, the C. I was green previously.

~~you removed it in the last push~~ I see, you removed another line. nvm.

l0rinc commented at 4:18 pm on February 11, 2026: contributor

ACK c1355493e2c26b613109bfac3dcd898b3acca75a

DrahtBot requested review from furszy on Feb 11, 2026

furszy commented at 4:26 pm on February 11, 2026: member

utACK c1355493e2c26b613109bfac3dcd898b3acca75a

musaHaruna commented at 2:07 pm on February 12, 2026: contributor

ACK c135549 — reviewed in the context of PR 34075

willcl-ark approved

willcl-ark commented at 10:54 am on February 16, 2026: member

ACK c1355493e2c26b613109bfac3dcd898b3acca75a

Seperating unrelated concerns (feerate and formatting) out makes sense. No stale FeeEstimateMode::BTC_KVB/SAT_VB references remaining.

fees: refactor: separate feerate format from fee estimate mode #34552

Motivation

Changes in this PR:

Reviews

Conflicts