From 74245c20e05e6fb5844a9bdfca489f107c6c8aca Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Fri, 26 Jun 2026 21:11:21 +0100 Subject: [PATCH] fees: split wallet and estimator fee reasons The block policy estimator's FeeReason enum mixed two unrelated concerns: the threshold that produced an estimateSmartFee result (NONE, HALF_ESTIMATE, ...) and the reason the wallet selected a fee rate (FALLBACK, MEMPOOL_MIN, REQUIRED). Split them so each layer owns the reasons it reports: - Add a wallet-facing FeeReason enum with the reasons the wallet can select a fee rate: FEE_RATE_ESTIMATOR, MEMPOOL_MIN, USER_SPECIFIED, FALLBACK, and REQUIRED. - Rename the estimator enum to BlockPolicyEstimateReason and narrow it to estimator reasons: NONE, HALF_ESTIMATE, FULL_ESTIMATE, DOUBLE_ESTIMATE, and CONSERVATIVE. - Return wallet fee selection metadata through MinimumFeeRateResult instead of exposing FeeCalculation to wallet callers. The returned target is now optional and is only set for fee rate estimator results. Flatten GetMinimumFeeRate() with early returns while preserving the fee selection order: user feerate still only applies the required-fee check, while smart-fee results keep fallback, mempool-min, and required fallbacks. The returned target is cleared for fallback, mempool-min, and required results. Replace the CreateTransactionInternal log with a simpler message that does not depend on estimateSmartFee internals. Detailed estimator logging will be added in a follow-up commit. --- src/common/messages.cpp | 24 +++++-- src/common/messages.h | 3 +- src/interfaces/wallet.h | 3 +- src/policy/fees/block_policy_estimator.cpp | 8 +-- src/policy/fees/block_policy_estimator.h | 7 +- src/qt/sendcoinsdialog.cpp | 10 ++- src/test/fuzz/fees.cpp | 6 +- src/util/fees.h | 9 +++ src/wallet/coincontrol.h | 1 - src/wallet/feebumper.cpp | 3 +- src/wallet/fees.cpp | 82 +++++++++++++--------- src/wallet/fees.h | 9 ++- src/wallet/interfaces.cpp | 12 ++-- src/wallet/rpc/spend.cpp | 13 ++-- src/wallet/spend.cpp | 20 ++---- src/wallet/spend.h | 1 - src/wallet/test/fuzz/fees.cpp | 23 ++++-- src/wallet/test/spend_tests.cpp | 1 - src/wallet/types.h | 23 ++++-- 19 files changed, 157 insertions(+), 101 deletions(-) diff --git a/src/common/messages.cpp b/src/common/messages.cpp index e558442f427..ae3672bffb2 100644 --- a/src/common/messages.cpp +++ b/src/common/messages.cpp @@ -28,12 +28,9 @@ namespace common { std::string StringForFeeReason(FeeReason reason) { static const std::map fee_reason_strings = { - {FeeReason::NONE, "None"}, - {FeeReason::HALF_ESTIMATE, "Half Target 60% Threshold"}, - {FeeReason::FULL_ESTIMATE, "Target 85% Threshold"}, - {FeeReason::DOUBLE_ESTIMATE, "Double Target 95% Threshold"}, - {FeeReason::CONSERVATIVE, "Conservative Double Target longer horizon"}, + {FeeReason::FEE_RATE_ESTIMATOR, "Fee Rate Estimator"}, {FeeReason::MEMPOOL_MIN, "Mempool Min Fee"}, + {FeeReason::USER_SPECIFIED, "User Specified Fee"}, {FeeReason::FALLBACK, "Fallback fee"}, {FeeReason::REQUIRED, "Minimum Required Fee"}, }; @@ -44,6 +41,23 @@ std::string StringForFeeReason(FeeReason reason) return reason_string->second; } +std::string StringForBlockPolicyEstimateReason(BlockPolicyEstimateReason reason) +{ + switch (reason) { + case BlockPolicyEstimateReason::NONE: + return "None"; + case BlockPolicyEstimateReason::HALF_ESTIMATE: + return "Half Target 60% Threshold"; + case BlockPolicyEstimateReason::FULL_ESTIMATE: + return "Target 85% Threshold"; + case BlockPolicyEstimateReason::DOUBLE_ESTIMATE: + return "Double Target 95% Threshold"; + case BlockPolicyEstimateReason::CONSERVATIVE: + return "Conservative Double Target longer horizon"; + } // no default case, so the compiler can warn about missing cases + assert(false); +} + const std::vector>& FeeModeMap() { static const std::vector> FEE_MODES = { diff --git a/src/common/messages.h b/src/common/messages.h index 6dc3212f8c9..4b7e42d953c 100644 --- a/src/common/messages.h +++ b/src/common/messages.h @@ -18,7 +18,7 @@ struct bilingual_str; enum class FeeEstimateMode; enum class FeeReason; - +enum class BlockPolicyEstimateReason; namespace node { enum class TransactionError; } // namespace node @@ -28,6 +28,7 @@ enum class PSBTError; bool FeeModeFromString(std::string_view mode_string, FeeEstimateMode& fee_estimate_mode); std::string StringForFeeReason(FeeReason reason); +std::string StringForBlockPolicyEstimateReason(BlockPolicyEstimateReason reason); std::string FeeModes(const std::string& delimiter); std::string FeeModeInfo(std::pair& mode); std::string FeeModesDetail(std::string default_info); diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index f78e3da03dd..d34fc62aca8 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -242,7 +243,7 @@ public: //! Get minimum fee. virtual CAmount getMinimumFee(unsigned int tx_bytes, const wallet::CCoinControl& coin_control, - int* returned_target, + std::optional* returned_target, FeeReason* reason) = 0; //! Get tx confirm target. diff --git a/src/policy/fees/block_policy_estimator.cpp b/src/policy/fees/block_policy_estimator.cpp index 1434c5de070..f4ed120eecc 100644 --- a/src/policy/fees/block_policy_estimator.cpp +++ b/src/policy/fees/block_policy_estimator.cpp @@ -919,7 +919,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation double halfEst = estimateCombinedFee(confTarget/2, HALF_SUCCESS_PCT, true, &tempResult); if (feeCalc) { feeCalc->est = tempResult; - feeCalc->reason = FeeReason::HALF_ESTIMATE; + feeCalc->reason = BlockPolicyEstimateReason::HALF_ESTIMATE; } median = halfEst; double actualEst = estimateCombinedFee(confTarget, SUCCESS_PCT, true, &tempResult); @@ -927,7 +927,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation median = actualEst; if (feeCalc) { feeCalc->est = tempResult; - feeCalc->reason = FeeReason::FULL_ESTIMATE; + feeCalc->reason = BlockPolicyEstimateReason::FULL_ESTIMATE; } } double doubleEst = estimateCombinedFee(2 * confTarget, DOUBLE_SUCCESS_PCT, !conservative, &tempResult); @@ -935,7 +935,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation median = doubleEst; if (feeCalc) { feeCalc->est = tempResult; - feeCalc->reason = FeeReason::DOUBLE_ESTIMATE; + feeCalc->reason = BlockPolicyEstimateReason::DOUBLE_ESTIMATE; } } @@ -945,7 +945,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation median = consEst; if (feeCalc) { feeCalc->est = tempResult; - feeCalc->reason = FeeReason::CONSERVATIVE; + feeCalc->reason = BlockPolicyEstimateReason::CONSERVATIVE; } } } diff --git a/src/policy/fees/block_policy_estimator.h b/src/policy/fees/block_policy_estimator.h index a87970cd4dc..cd6961c4013 100644 --- a/src/policy/fees/block_policy_estimator.h +++ b/src/policy/fees/block_policy_estimator.h @@ -56,15 +56,12 @@ inline constexpr auto ALL_FEE_ESTIMATE_HORIZONS = std::array{ std::string StringForFeeEstimateHorizon(FeeEstimateHorizon horizon); /* Enumeration of reason for returned fee estimate */ -enum class FeeReason { +enum class BlockPolicyEstimateReason { NONE, HALF_ESTIMATE, FULL_ESTIMATE, DOUBLE_ESTIMATE, CONSERVATIVE, - MEMPOOL_MIN, - FALLBACK, - REQUIRED, }; /* Used to return detailed information about a feerate bucket */ @@ -90,7 +87,7 @@ struct EstimationResult struct FeeCalculation { EstimationResult est; - FeeReason reason = FeeReason::NONE; + BlockPolicyEstimateReason reason = BlockPolicyEstimateReason::NONE; int desiredTarget = 0; int returnedTarget = 0; unsigned int best_height{0}; diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 7c8c04366f7..9bc0ff48df0 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -21,17 +21,18 @@ #include #include #include -#include #include #include #include #include +#include #include #include #include #include #include +#include #include #include @@ -837,7 +838,7 @@ void SendCoinsDialog::updateSmartFeeLabel() return; updateCoinControlState(); m_coin_control->m_feerate.reset(); // Explicitly use only fee estimation rate for smart fee labels - int returned_target; + std::optional returned_target; FeeReason reason; CFeeRate feeRate = CFeeRate(model->wallet().getMinimumFee(1000, *m_coin_control, &returned_target, &reason)); @@ -855,7 +856,10 @@ void SendCoinsDialog::updateSmartFeeLabel() else { ui->labelSmartFee2->hide(); - ui->labelFeeEstimation->setText(tr("Estimated to begin confirmation within %n block(s).", "", returned_target)); + ui->labelFeeEstimation->setText(""); + if (returned_target) { + ui->labelFeeEstimation->setText(tr("Estimated to begin confirmation within %n block(s).", "", *returned_target)); + } ui->fallbackFeeWarningLabel->setVisible(false); } diff --git a/src/test/fuzz/fees.cpp b/src/test/fuzz/fees.cpp index 1940e342c67..5c5f14ffff6 100644 --- a/src/test/fuzz/fees.cpp +++ b/src/test/fuzz/fees.cpp @@ -8,12 +8,14 @@ #include #include #include +#include #include #include #include using common::StringForFeeReason; +using common::StringForBlockPolicyEstimateReason; FUZZ_TARGET(fees) { @@ -26,6 +28,8 @@ FUZZ_TARGET(fees) const CAmount rounded_fee = fee_filter_rounder.round(current_minimum_fee); assert(MoneyRange(rounded_fee)); } - const FeeReason fee_reason = fuzzed_data_provider.PickValueInArray({FeeReason::NONE, FeeReason::HALF_ESTIMATE, FeeReason::FULL_ESTIMATE, FeeReason::DOUBLE_ESTIMATE, FeeReason::CONSERVATIVE, FeeReason::MEMPOOL_MIN, FeeReason::FALLBACK, FeeReason::REQUIRED}); + const FeeReason fee_reason = fuzzed_data_provider.PickValueInArray({FeeReason::FEE_RATE_ESTIMATOR, FeeReason::MEMPOOL_MIN, FeeReason::USER_SPECIFIED, FeeReason::FALLBACK, FeeReason::REQUIRED}); (void)StringForFeeReason(fee_reason); + const BlockPolicyEstimateReason block_policy_fee_reason = fuzzed_data_provider.PickValueInArray({BlockPolicyEstimateReason::NONE, BlockPolicyEstimateReason::HALF_ESTIMATE, BlockPolicyEstimateReason::FULL_ESTIMATE, BlockPolicyEstimateReason::DOUBLE_ESTIMATE, BlockPolicyEstimateReason::CONSERVATIVE}); + (void)StringForBlockPolicyEstimateReason(block_policy_fee_reason); } diff --git a/src/util/fees.h b/src/util/fees.h index 6edcb772d24..85c3d59d462 100644 --- a/src/util/fees.h +++ b/src/util/fees.h @@ -12,4 +12,13 @@ enum class FeeEstimateMode { CONSERVATIVE, //!< Force estimateSmartFee to use conservative estimates }; +/* Used to determine the reason a wallet selected a transaction fee rate */ +enum class FeeReason { + FEE_RATE_ESTIMATOR, + MEMPOOL_MIN, + USER_SPECIFIED, + FALLBACK, + REQUIRED, +}; + #endif // BITCOIN_UTIL_FEES_H diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index d30cbe1c519..53fc0e8fcf0 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -7,7 +7,6 @@ #include #include -#include #include #include