← Watch feed
refactor: fees: split fee rate format from fee estimate mode
Public commit record
95/100 · Strong
What the developer wrote
Authored by ismaelsadeeq
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.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides detailed explanatory context✓ Mentions testing or verification
What changed, and why it matters
This is a code cleanup change that separates how fee rates are displayed (BTC/kvB versus sat/vB) from how fees are estimated. It introduces a new FeeRateFormat enum and removes display-only values from the FeeEstimateMode enum. There is no security fix or behavior change visible in the diff.
Recommended action
No security action needed. Treat as ordinary refactoring review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors fee-rate string formatting. It creates a FeeRateFormat enum in src/policy/feerate.h, changes CFeeRate::ToString() to take FeeRateFormat instead of FeeEstimateMode, removes BTC_KVB and SAT_VB from FeeEstimateMode, and updates call sites/tests accordingly. The switch in ToString() now covers all enum values and asserts on unhandled cases. No functional fee calculation, validation, or consensus logic is modified.
Changed components
src/policy/feerate.cppsrc/policy/feerate.hsrc/util/fees.hsrc/common/messages.cppsrc/rpc/fees.cppsrc/wallet/rpc/spend.cppsrc/wallet/spend.cppsrc/wallet/coincontrol.hsrc/test/amount_tests.cppsrc/test/fuzz/string.cppInspect captured patch +24 / −19
diff --git a/src/common/messages.cpp b/src/common/messages.cpp
index 123db93c..4dfee3a5 100644
--- a/src/common/messages.cpp
+++ b/src/common/messages.cpp
@@ -4,11 +4,11 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <common/messages.h>
-
#include <common/types.h>
-#include <policy/fees/block_policy_estimator.h>
#include <node/types.h>
+#include <policy/fees/block_policy_estimator.h>
#include <tinyformat.h>
+#include <util/fees.h>
#include <util/strencodings.h>
#include <util/string.h>
#include <util/translation.h>
@@ -68,7 +68,6 @@ std::string FeeModeInfo(const std::pair<std::string, FeeEstimateMode>& mode, std
"less responsive to short-term drops in the prevailing fee market. This mode\n"
"potentially returns a higher fee rate estimate.\n", mode.first);
default:
- // Other modes apart from the ones handled are fee rate units; they should not be clarified.
assert(false);
}
}
diff --git a/src/policy/feerate.cpp b/src/policy/feerate.cpp
index f74da8a7..f62835ac 100644
--- a/src/policy/feerate.cpp
+++ b/src/policy/feerate.cpp
@@ -26,11 +26,12 @@ CAmount CFeeRate::GetFee(int32_t virtual_bytes) const
return nFee;
}
-std::string CFeeRate::ToString(const FeeEstimateMode& fee_estimate_mode) const
+std::string CFeeRate::ToString(FeeRateFormat fee_rate_format) const
{
- const CAmount feerate_per_kvb = GetFeePerK();
- switch (fee_estimate_mode) {
- case FeeEstimateMode::SAT_VB: return strprintf("%d.%03d %s/vB", feerate_per_kvb / 1000, feerate_per_kvb % 1000, CURRENCY_ATOM);
- default: return strprintf("%d.%08d %s/kvB", feerate_per_kvb / COIN, feerate_per_kvb % COIN, CURRENCY_UNIT);
- }
+ const CAmount feerate_per_kvb{GetFeePerK()};
+ switch (fee_rate_format) {
+ case FeeRateFormat::BTC_KVB: return strprintf("%d.%08d %s/kvB", feerate_per_kvb / COIN, feerate_per_kvb % COIN, CURRENCY_UNIT);
+ case FeeRateFormat::SAT_VB: return strprintf("%d.%03d %s/vB", feerate_per_kvb / 1000, feerate_per_kvb % 1000, CURRENCY_ATOM);
+ } // no default case, so the compiler can warn about missing cases
+ assert(false);
}
diff --git a/src/policy/feerate.h b/src/policy/feerate.h
index b89f2e1e..f6b49a14 100644
--- a/src/policy/feerate.h
+++ b/src/policy/feerate.h
@@ -18,6 +18,12 @@
const std::string CURRENCY_UNIT = "BTC"; // One formatted unit
const std::string CURRENCY_ATOM = "sat"; // One indivisible minimum value unit
+
+enum class FeeRateFormat {
+ BTC_KVB, //!< Use BTC/kvB fee rate unit
+ SAT_VB, //!< Use sat/vB fee rate unit
+};
+
/**
* Fee rate in satoshis per virtualbyte: CAmount / vB
* the feerate is represented internally as FeeFrac
@@ -66,7 +72,7 @@ public:
m_feerate = FeePerVSize(GetFeePerK() + a.GetFeePerK(), 1000);
return *this;
}
- std::string ToString(const FeeEstimateMode& fee_estimate_mode = FeeEstimateMode::BTC_KVB) const;
+ std::string ToString(FeeRateFormat fee_rate_format = FeeRateFormat::BTC_KVB) const;
friend CFeeRate operator*(const CFeeRate& f, int a) { return CFeeRate(a * f.m_feerate.fee, f.m_feerate.size); }
friend CFeeRate operator*(int a, const CFeeRate& f) { return CFeeRate(a * f.m_feerate.fee, f.m_feerate.size); }
diff --git a/src/rpc/fees.cpp b/src/rpc/fees.cpp
index 174217fc..9bad6f4a 100644
--- a/src/rpc/fees.cpp
+++ b/src/rpc/fees.cpp
@@ -15,6 +15,7 @@
#include <rpc/util.h>
#include <txmempool.h>
#include <univalue.h>
+#include <util/fees.h>
#include <validationinterface.h>
#include <algorithm>
diff --git a/src/test/amount_tests.cpp b/src/test/amount_tests.cpp
index b500c968..e1630b41 100644
--- a/src/test/amount_tests.cpp
+++ b/src/test/amount_tests.cpp
@@ -138,8 +138,8 @@ BOOST_AUTO_TEST_CASE(ToStringTest)
CFeeRate feeRate;
feeRate = CFeeRate(1);
BOOST_CHECK_EQUAL(feeRate.ToString(), "0.00000001 BTC/kvB");
- BOOST_CHECK_EQUAL(feeRate.ToString(FeeEstimateMode::BTC_KVB), "0.00000001 BTC/kvB");
- BOOST_CHECK_EQUAL(feeRate.ToString(FeeEstimateMode::SAT_VB), "0.001 sat/vB");
+ BOOST_CHECK_EQUAL(feeRate.ToString(FeeRateFormat::BTC_KVB), "0.00000001 BTC/kvB");
+ BOOST_CHECK_EQUAL(feeRate.ToString(FeeRateFormat::SAT_VB), "0.001 sat/vB");
}
BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp
index 5d59eb34..d3156336 100644
--- a/src/test/fuzz/string.cpp
+++ b/src/test/fuzz/string.cpp
@@ -22,6 +22,7 @@
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h>
+#include <util/fees.h>
#include <util/strencodings.h>
#include <util/string.h>
#include <util/translation.h>
@@ -34,8 +35,6 @@
#include <string>
#include <vector>
-enum class FeeEstimateMode;
-
using common::AmountErrMsg;
using common::AmountHighWarn;
using common::FeeModeFromString;
diff --git a/src/util/fees.h b/src/util/fees.h
index 25ca246e..6edcb772 100644
--- a/src/util/fees.h
+++ b/src/util/fees.h
@@ -10,8 +10,6 @@ enum class FeeEstimateMode {
UNSET, //!< Use default settings based on other criteria
ECONOMICAL, //!< Force estimateSmartFee to use non-conservative estimates
CONSERVATIVE, //!< Force estimateSmartFee to use conservative estimates
- BTC_KVB, //!< Use BTC/kvB fee rate unit
- SAT_VB, //!< Use sat/vB fee rate unit
};
#endif // BITCOIN_UTIL_FEES_H
diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h
index ead3307a..27cbd387 100644
--- a/src/wallet/coincontrol.h
+++ b/src/wallet/coincontrol.h
@@ -11,6 +11,7 @@
#include <primitives/transaction.h>
#include <script/keyorigin.h>
#include <script/signingprovider.h>
+#include <util/fees.h>
#include <algorithm>
#include <map>
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 64cf95a4..68e469d8 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -1008,7 +1008,7 @@ static std::vector<RPCArg> OutputsDoc()
static RPCHelpMan bumpfee_helper(std::string method_name)
{
const bool want_psbt = method_name == "psbtbumpfee";
- const std::string incremental_fee{CFeeRate(DEFAULT_INCREMENTAL_RELAY_FEE).ToString(FeeEstimateMode::SAT_VB)};
+ const std::string incremental_fee{CFeeRate(DEFAULT_INCREMENTAL_RELAY_FEE).ToString(FeeRateFormat::SAT_VB)};
return RPCHelpMan{method_name,
"Bumps the fee of a transaction T, replacing it with a new transaction B.\n"
@@ -1483,7 +1483,7 @@ RPCHelpMan sendall()
// Do not, ever, assume that it's fine to change the fee rate if the user has explicitly
// provided one
if (coin_control.m_feerate && fee_rate > *coin_control.m_feerate) {
- throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Fee rate (%s) is lower than the minimum fee rate setting (%s)", coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), fee_rate.ToString(FeeEstimateMode::SAT_VB)));
+ throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Fee rate (%s) is lower than the minimum fee rate setting (%s)", coin_control.m_feerate->ToString(FeeRateFormat::SAT_VB), fee_rate.ToString(FeeRateFormat::SAT_VB)));
}
if (fee_calc_out.reason == FeeReason::FALLBACK && !pwallet->m_allow_fallback_fee) {
// eventually allow a fallback fee
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index edde9234..9ba507f4 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -1153,7 +1153,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
// Do not, ever, assume that it's fine to change the fee rate if the user has explicitly
// provided one
if (coin_control.m_feerate && coin_selection_params.m_effective_feerate > *coin_control.m_feerate) {
- return util::Error{strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB))};
+ return util::Error{strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeRateFormat::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeRateFormat::SAT_VB))};
}
if (feeCalc.reason == FeeReason::FALLBACK && !wallet.m_allow_fallback_fee) {
// eventually allow a fallback fee
Risk score
Our methodology →Why this scored 14/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.