highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-01-21 07:39:08 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#32750: refactor: CFeeRate encapsulates FeeFrac internally

Browse Source 
d3b8a54a81 Refactor CFeeRate to use FeeFrac internally (Pol Espinasa)

Pull request description:

  The `FeeFrac` type represents a fraction, intended to be used for `sats/vbyte` or `sats/WU`. It was added to improve accuracy when evaluating fee rates in cluster mempool. [1]
  But it can also be used to fix the precision issues that the current `CFeeRate` class has now.

  At the moment, `CFeeRate` handles the fee rate as  satoshis per kilovirtualbyte: `CAmount / kvB` using an integer.
  This PR fix `CFeeRate` precision issues by encapsulating `FeeFrac` internally keeping backwards compatibility.

  This PR can also be used as a based to use multiple units on RPC calls as detailed in this issue [2].

  Some previous discussions:
  [1] https://github.com/bitcoin/bitcoin/pull/30535
  [2] https://github.com/bitcoin/bitcoin/issues/32093

ACKs for top commit:
  achow101:
    ACK d3b8a54a81
  murchandamus:
    code review, lightly tested ACK d3b8a54a81
  ismaelsadeeq:
    re-ACK d3b8a54a81 📦
  theStack:
    Code-review ACK d3b8a54a81

Tree-SHA512: 5a8149d81e82ad4e60a0e76ff6a82a5b1c4e212cf5156c1cdd16bf9acbb351e7be458eac3f0a2ae89107f331062b299c1d9ca649d3b820ad0b68e6d1a14292e5
This commit is contained in:
Ava Chow
2025-08-08 18:11:05 -07:00
parent f679bad605 d3b8a54a81
commit daca51bf80
11 changed files with 66 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
src/policy/feerate.cpp
View File

@@ -1,5 +1,5 @@
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2022 The Bitcoin Core developers
// Copyright (c) 2009-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@@ -7,39 +7,30 @@
#include <policy/feerate.h>
#include <tinyformat.h>
#include <cmath>
CFeeRate::CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes)
CFeeRate::CFeeRate(const CAmount& nFeePaid, int32_t virtual_bytes)
{
const int64_t nSize{num_bytes};
if (nSize > 0) {
nSatoshisPerK = nFeePaid * 1000 / nSize;
if (virtual_bytes > 0) {
m_feerate = FeePerVSize(nFeePaid, virtual_bytes);
} else {
nSatoshisPerK = 0;
m_feerate = FeePerVSize();
}
}
CAmount CFeeRate::GetFee(uint32_t num_bytes) const
CAmount CFeeRate::GetFee(int32_t virtual_bytes) const
{
const int64_t nSize{num_bytes};
// Be explicit that we're converting from a double to int64_t (CAmount) here.
// We've previously had issues with the silent double->int64_t conversion.
CAmount nFee{static_cast<CAmount>(std::ceil(nSatoshisPerK * nSize / 1000.0))};
if (nFee == 0 && nSize != 0) {
if (nSatoshisPerK > 0) nFee = CAmount(1);
if (nSatoshisPerK < 0) nFee = CAmount(-1);
}
Assume(virtual_bytes >= 0);
if (m_feerate.IsEmpty()) { return CAmount(0);}
CAmount nFee = CAmount(m_feerate.EvaluateFeeUp(virtual_bytes));
if (nFee == 0 && virtual_bytes != 0 && m_feerate.fee < 0) return CAmount(-1);
return nFee;
}
std::string CFeeRate::ToString(const FeeEstimateMode& fee_estimate_mode) const
{
const CAmount feerate_per_kvb = GetFeePerK();
switch (fee_estimate_mode) {
case FeeEstimateMode::SAT_VB: return strprintf("%d.%03d %s/vB", nSatoshisPerK / 1000, nSatoshisPerK % 1000, CURRENCY_ATOM);
default: return strprintf("%d.%08d %s/kvB", nSatoshisPerK / COIN, nSatoshisPerK % COIN, CURRENCY_UNIT);
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);
}
}

52
src/policy/feerate.h
View File

@@ -1,5 +1,5 @@
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2022 The Bitcoin Core developers
// Copyright (c) 2009-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@@ -8,6 +8,7 @@
#include <consensus/amount.h>
#include <serialize.h>
#include <util/feefrac.h>
#include <cstdint>
@@ -27,52 +28,59 @@ enum class FeeEstimateMode {
};
/**
* Fee rate in satoshis per kilovirtualbyte: CAmount / kvB
* Fee rate in satoshis per virtualbyte: CAmount / vB
* the feerate is represented internally as FeeFrac
*/
class CFeeRate
{
private:
/** Fee rate in sat/kvB (satoshis per 1000 virtualbytes) */
CAmount nSatoshisPerK;
/** Fee rate in sats/vB (satoshis per N virtualbytes) */
FeePerVSize m_feerate;
public:
/** Fee rate of 0 satoshis per kvB */
CFeeRate() : nSatoshisPerK(0) { }
/** Fee rate of 0 satoshis per 0 vB */
CFeeRate() = default;
template<std::integral I> // Disallow silent float -> int conversion
explicit CFeeRate(const I _nSatoshisPerK): nSatoshisPerK(_nSatoshisPerK) {
}
explicit CFeeRate(const I m_feerate_kvb) : m_feerate(FeePerVSize(m_feerate_kvb, 1000)) {}
/**
* Construct a fee rate from a fee in satoshis and a vsize in vB.
*
* param@[in] nFeePaid The fee paid by a transaction, in satoshis
* param@[in] num_bytes The vsize of a transaction, in vbytes
* param@[in] virtual_bytes The vsize of a transaction, in vbytes
*
* Passing any virtual_bytes less than or equal to 0 will result in 0 fee rate per 0 size.
*/
CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes);
CFeeRate(const CAmount& nFeePaid, int32_t virtual_bytes);
/**
* Return the fee in satoshis for the given vsize in vbytes.
* If the calculated fee would have fractional satoshis, then the
* returned fee will always be rounded up to the nearest satoshi.
*/
CAmount GetFee(uint32_t num_bytes) const;
CAmount GetFee(int32_t virtual_bytes) const;
/**
* Return the fee in satoshis for a vsize of 1000 vbytes
*/
CAmount GetFeePerK() const { return nSatoshisPerK; }
friend bool operator<(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK < b.nSatoshisPerK; }
friend bool operator>(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK > b.nSatoshisPerK; }
friend bool operator==(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK == b.nSatoshisPerK; }
friend bool operator<=(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK <= b.nSatoshisPerK; }
friend bool operator>=(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK >= b.nSatoshisPerK; }
friend bool operator!=(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK != b.nSatoshisPerK; }
CFeeRate& operator+=(const CFeeRate& a) { nSatoshisPerK += a.nSatoshisPerK; return *this; }
CAmount GetFeePerK() const { return CAmount(m_feerate.EvaluateFeeDown(1000)); }
friend std::weak_ordering operator<=>(const CFeeRate& a, const CFeeRate& b) noexcept
{
return FeeRateCompare(a.m_feerate, b.m_feerate);
}
friend bool operator==(const CFeeRate& a, const CFeeRate& b) noexcept
{
return FeeRateCompare(a.m_feerate, b.m_feerate) == std::weak_ordering::equivalent;
}
CFeeRate& operator+=(const CFeeRate& a) {
m_feerate = FeePerVSize(GetFeePerK() + a.GetFeePerK(), 1000);
return *this;
}
std::string ToString(const FeeEstimateMode& fee_estimate_mode = FeeEstimateMode::BTC_KVB) const;
friend CFeeRate operator*(const CFeeRate& f, int a) { return CFeeRate(a * f.nSatoshisPerK); }
friend CFeeRate operator*(int a, const CFeeRate& f) { return CFeeRate(a * f.nSatoshisPerK); }
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); }
SERIALIZE_METHODS(CFeeRate, obj) { READWRITE(obj.nSatoshisPerK); }
SERIALIZE_METHODS(CFeeRate, obj) { READWRITE(obj.m_feerate.fee, obj.m_feerate.size); }
};
#endif // BITCOIN_POLICY_FEERATE_H

17
src/test/amount_tests.cpp
View File

@@ -1,4 +1,4 @@
// Copyright (c) 2016-2021 The Bitcoin Core developers
// Copyright (c) 2016-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@@ -73,18 +73,21 @@ BOOST_AUTO_TEST_CASE(GetFeeTest)
BOOST_CHECK(CFeeRate(CAmount(-1), 0) == CFeeRate(0));
BOOST_CHECK(CFeeRate(CAmount(0), 0) == CFeeRate(0));
BOOST_CHECK(CFeeRate(CAmount(1), 0) == CFeeRate(0));
BOOST_CHECK(CFeeRate(CAmount(1), -1000) == CFeeRate(0));
// default value
BOOST_CHECK(CFeeRate(CAmount(-1), 1000) == CFeeRate(-1));
BOOST_CHECK(CFeeRate(CAmount(0), 1000) == CFeeRate(0));
BOOST_CHECK(CFeeRate(CAmount(1), 1000) == CFeeRate(1));
// lost precision (can only resolve satoshis per kB)
BOOST_CHECK(CFeeRate(CAmount(1), 1001) == CFeeRate(0));
BOOST_CHECK(CFeeRate(CAmount(2), 1001) == CFeeRate(1));
// Previously, precision was limited to three decimal digits
// due to only supporting satoshis per kB, so CFeeRate(CAmount(1), 1001) was equal to CFeeRate(0)
// Since #32750, higher precision is maintained.
BOOST_CHECK(CFeeRate(CAmount(1), 1001) > CFeeRate(0) && CFeeRate(CAmount(1), 1001) < CFeeRate(1));
BOOST_CHECK(CFeeRate(CAmount(2), 1001) > CFeeRate(1) && CFeeRate(CAmount(2), 1001) < CFeeRate(2));
// some more integer checks
BOOST_CHECK(CFeeRate(CAmount(26), 789) == CFeeRate(32));
BOOST_CHECK(CFeeRate(CAmount(27), 789) == CFeeRate(34));
BOOST_CHECK(CFeeRate(CAmount(26), 789) > CFeeRate(32) && CFeeRate(CAmount(26), 789) < CFeeRate(33));
BOOST_CHECK(CFeeRate(CAmount(27), 789) > CFeeRate(34) && CFeeRate(CAmount(27), 789) < CFeeRate(35));
// Maximum size in bytes, should not crash
CFeeRate(MAX_MONEY, std::numeric_limits<uint32_t>::max()).GetFeePerK();
CFeeRate(MAX_MONEY, std::numeric_limits<int32_t>::max()).GetFeePerK();
// check multiplication operator
// check multiplying by zero

4
src/test/fuzz/fee_rate.cpp
View File

@@ -1,4 +1,4 @@
// Copyright (c) 2020-2021 The Bitcoin Core developers
// Copyright (c) 2020-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@@ -20,7 +20,7 @@ FUZZ_TARGET(fee_rate)
const CFeeRate fee_rate{satoshis_per_k};
(void)fee_rate.GetFeePerK();
const auto bytes = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
const auto bytes = fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(0, std::numeric_limits<int32_t>::max());
if (!MultiplicationOverflow(int64_t{bytes}, satoshis_per_k)) {
(void)fee_rate.GetFee(bytes);
}

2
src/test/fuzz/feefrac.cpp
View File

@@ -223,7 +223,7 @@ FUZZ_TARGET(feefrac_mul_div)
if (mul64 < std::numeric_limits<int64_t>::max() / 1000 &&
mul64 > std::numeric_limits<int64_t>::min() / 1000 &&
quot_abs < arith_uint256{std::numeric_limits<int64_t>::max() / 1000}) {
CFeeRate feerate(mul64, (uint32_t)div);
CFeeRate feerate(mul64, div);
CAmount feerate_fee{feerate.GetFee(mul32)};
auto allowed_gap = static_cast<int64_t>(mul32 / 1000 + 3 + round_down);
assert(feerate_fee - res_fee >= -allowed_gap);

2
src/test/fuzz/fees.cpp
View File

@@ -1,4 +1,4 @@
// Copyright (c) 2020-2021 The Bitcoin Core developers
// Copyright (c) 2020-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

6
src/validation.cpp
View File

@@ -1406,7 +1406,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
auto iter = m_pool.GetIter(ws.m_ptx->GetHash());
Assume(iter.has_value());
const auto effective_feerate = args.m_package_feerates ? ws.m_package_feerate :
CFeeRate{ws.m_modified_fees, static_cast<uint32_t>(ws.m_vsize)};
CFeeRate{ws.m_modified_fees, static_cast<int32_t>(ws.m_vsize)};
const auto effective_feerate_wtxids = args.m_package_feerates ? all_package_wtxids :
std::vector<Wtxid>{ws.m_ptx->GetWitnessHash()};
results.emplace(ws.m_ptx->GetWitnessHash(),
@@ -1470,7 +1470,7 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
if (!ConsensusScriptChecks(args, ws)) return MempoolAcceptResult::Failure(ws.m_state);
const CFeeRate effective_feerate{ws.m_modified_fees, static_cast<uint32_t>(ws.m_vsize)};
const CFeeRate effective_feerate{ws.m_modified_fees, static_cast<int32_t>(ws.m_vsize)};
// Tx was accepted, but not added
if (args.m_test_accept) {
return MempoolAcceptResult::Success(std::move(m_subpackage.m_replaced_transactions), ws.m_vsize,
@@ -1626,7 +1626,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
}
if (args.m_test_accept) {
const auto effective_feerate = args.m_package_feerates ? ws.m_package_feerate :
CFeeRate{ws.m_modified_fees, static_cast<uint32_t>(ws.m_vsize)};
CFeeRate{ws.m_modified_fees, static_cast<int32_t>(ws.m_vsize)};
const auto effective_feerate_wtxids = args.m_package_feerates ? all_package_wtxids :
std::vector<Wtxid>{ws.m_ptx->GetWitnessHash()};
results.emplace(ws.m_ptx->GetWitnessHash(),

6
src/wallet/fees.cpp
View File

@@ -1,5 +1,5 @@
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2022 The Bitcoin Core developers
// Copyright (c) 2009-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@@ -12,13 +12,13 @@
namespace wallet {
CAmount GetRequiredFee(const CWallet& wallet, unsigned int nTxBytes)
{
return GetRequiredFeeRate(wallet).GetFee(nTxBytes);
return GetRequiredFeeRate(wallet).GetFee(static_cast<int32_t>(nTxBytes));
}
CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, FeeCalculation* feeCalc)
{
return GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes);
return GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(static_cast<int32_t>(nTxBytes));
}
CFeeRate GetRequiredFeeRate(const CWallet& wallet)

2
src/wallet/fees.h
View File

@@ -1,5 +1,5 @@
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2021 The Bitcoin Core developers
// Copyright (c) 2009-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

2
src/wallet/test/coinselector_tests.cpp
View File

@@ -244,7 +244,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
coin_selection_params_bnb.m_long_term_feerate = CFeeRate(3000);
// Add selectable outputs, increasing their raw amounts by their input fee to make the effective value equal to the raw amount
CAmount input_fee = coin_selection_params_bnb.m_effective_feerate.GetFee(/*num_bytes=*/68); // bech32 input size (default test output type)
CAmount input_fee = coin_selection_params_bnb.m_effective_feerate.GetFee(/*virtual_bytes=*/68); // bech32 input size (default test output type)
add_coin(available_coins, *wallet, 10 * CENT + input_fee, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
add_coin(available_coins, *wallet, 9 * CENT + input_fee, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
add_coin(available_coins, *wallet, 1 * CENT + input_fee, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);

3
src/wallet/test/fuzz/fees.cpp
View File

@@ -45,8 +45,7 @@ FUZZ_TARGET(wallet_fees, .init = initialize_setup)
}
(void)GetDiscardRate(wallet);
const auto tx_bytes{fuzzed_data_provider.ConsumeIntegral<unsigned int>()};
const auto tx_bytes{fuzzed_data_provider.ConsumeIntegralInRange(0, std::numeric_limits<int32_t>::max())};
if (fuzzed_data_provider.ConsumeBool()) {
wallet.m_pay_tx_fee = CFeeRate{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)};
wallet.m_min_fee = CFeeRate{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)};