Repository: bitcoin/bitcoin
File: src/policy/feerate.cpp (C++)

Code before:
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2022 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <consensus/amount.h>
#include <policy/feerate.h>
#include <tinyformat.h>

#include <cmath>

CFeeRate::CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes)
{
    const int64_t nSize{num_bytes};

    if (nSize > 0) {
        nSatoshisPerK = nFeePaid * 1000 / nSize;
    } else {
        nSatoshisPerK = 0;
    }
}

CAmount CFeeRate::GetFee(uint32_t num_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);
    }

    return nFee;
}

Review comment (bug):
The reason that the fuzz tests fail before changing this to `uint32_t` (e.g. this commit: https://github.com/davidgumberg/bitcoin/commit/01a4557b10b31e620716cff04c554e6f1a95653a)  is because `CFeeRate::GetFee` takes an unsigned int, but then passes it  to `EvaluateFeeUp()` which implicitly converts `at_size` to a signed value, and large examples of `uint32_t` values become negative integers, this results in `EvaluateFee()` getting passed a negative number and asserting.

Wouldn't it be better to change the `FeeFrac::EvaluateFee*()` functions to take unsigned integers, e.g.:  

```diff
diff --git a/src/util/feefrac.h b/src/util/feefrac.h
index 7577107e8c..9d034c6791 100644
--- a/src/util/feefrac.h
+++ b/src/util/feefrac.h
@@ -199,10 +199,9 @@ struct FeeFrac
      * is guaranteed to be the case when 0 <= at_size <= this->size.
      */
     template<bool RoundDown>
-    int64_t EvaluateFee(int32_t at_size) const noexcept
+    int64_t EvaluateFee(uint32_t at_size) const noexcept
     {
         Assume(size > 0);
-        Assume(at_size >= 0);
         if (fee >= 0 && fee < 0x200000000) [[likely]] {
             // Common case where (this->fee * at_size) is guaranteed to fit in a uint64_t.
             if constexpr (RoundDown) {
@@ -218,9 +217,9 @@ struct FeeFrac

 public:
     /** Compute the fee for a given size `at_size` using this object's feerate, rounding down. */
-    int64_t EvaluateFeeDown(int32_t at_size) const noexcept { return EvaluateFee<true>(at_size); }
+    int64_t EvaluateFeeDown(uint32_t at_size) const noexcept { return EvaluateFee<true>(at_size); }
     /** Compute the fee for a given size `at_size` using this object's feerate, rounding up. */
-    int64_t EvaluateFeeUp(int32_t at_size) const noexcept { return EvaluateFee<false>(at_size); }
+    int64_t EvaluateFeeUp(uint32_t at_size) const noexcept { return EvaluateFee<false>(at_size); }
 };

 /** Compare the feerate diagrams implied by the provided sorted chunks data.
 ```
 
Tested as follows:

```bash
cmake --preset libfuzzer && cmake --build build_fuzz -j $(nproc)
FUZZ=fee_rate_fuzz_target 
```

Code after:
// Copyright (c) 2009-2010 Satoshi Nakamoto
// 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.

#include <consensus/amount.h>
#include <policy/feerate.h>
#include <tinyformat.h>


CFeeRate::CFeeRate(const CAmount& nFeePaid, int32_t virtual_bytes)
{
    if (virtual_bytes > 0) {
        m_feerate = FeePerVSize(nFeePaid, virtual_bytes);
    } else {
        m_feerate = FeePerVSize();
    }
}

CAmount CFeeRate::GetFee(int32_t virtual_bytes) const
{
    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", 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);
    }
}