Merge #7796: [amount] Add support for negative fee rates

facf5a4 [amount] tests: Fix off-by-one mistake (MarcoFalke)
fa2da2c [amount] Add support for negative fee rates (MarcoFalke)
11114a6 [amount] test negative fee rates and full constructor (MarcoFalke)

3 files changed, 51 insertions, 9 deletions

diff --git a/src/amount.cpp b/src/amount.cpp
index 68806ff062..7b8618de33 100644
--- a/src/amount.cpp
+++ b/src/amount.cpp
@@ -9,20 +9,30 @@
 
 const std::string CURRENCY_UNIT = "BTC";
 
-CFeeRate::CFeeRate(const CAmount& nFeePaid, size_t nSize)
+CFeeRate::CFeeRate(const CAmount& nFeePaid, size_t nBytes_)
 {
+    assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
+    int64_t nSize = int64_t(nBytes_);
+
     if (nSize > 0)
-        nSatoshisPerK = nFeePaid*1000/nSize;
+        nSatoshisPerK = nFeePaid * 1000 / nSize;
     else
         nSatoshisPerK = 0;
 }
 
-CAmount CFeeRate::GetFee(size_t nSize) const
+CAmount CFeeRate::GetFee(size_t nBytes_) const
 {
+    assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
+    int64_t nSize = int64_t(nBytes_);
+
     CAmount nFee = nSatoshisPerK * nSize / 1000;
 
-    if (nFee == 0 && nSize != 0 && nSatoshisPerK > 0)
-        nFee = CAmount(1);
+    if (nFee == 0 && nSize != 0) {
+        if (nSatoshisPerK > 0)
+            nFee = CAmount(1);
+        if (nSatoshisPerK < 0)
+            nFee = CAmount(-1);
+    }
 
     return nFee;
 }
diff --git a/src/amount.h b/src/amount.h
index 9aba6525c7..5e52f37f23 100644
--- a/src/amount.h
+++ b/src/amount.h
@@ -11,6 +11,7 @@
 #include <stdlib.h>
 #include <string>
 
+/** Amount in satoshis (Can be negative) */
 typedef int64_t CAmount;
 
 static const CAmount COIN = 100000000;
@@ -30,22 +31,24 @@ extern const std::string CURRENCY_UNIT;
 static const CAmount MAX_MONEY = 21000000 * COIN;
 inline bool MoneyRange(const CAmount& nValue) { return (nValue >= 0 && nValue <= MAX_MONEY); }
 
-/** Type-safe wrapper class for fee rates
- * (how much to pay based on transaction size)
+/**
+ * Fee rate in satoshis per kilobyte: CAmount / kB
  */
 class CFeeRate
 {
 private:
     CAmount nSatoshisPerK; // unit is satoshis-per-1,000-bytes
 public:
+    /** Fee rate of 0 satoshis per kB */
     CFeeRate() : nSatoshisPerK(0) { }
     explicit CFeeRate(const CAmount& _nSatoshisPerK): nSatoshisPerK(_nSatoshisPerK) { }
-    CFeeRate(const CAmount& nFeePaid, size_t nSize);
+    /** Constructor for a fee rate in satoshis per kB. The size in bytes must not exceed (2^63 - 1)*/
+    CFeeRate(const CAmount& nFeePaid, size_t nBytes);
     CFeeRate(const CFeeRate& other) { nSatoshisPerK = other.nSatoshisPerK; }
     /**
      * Return the fee in satoshis for the given size in bytes.
      */
-    CAmount GetFee(size_t size) const;
+    CAmount GetFee(size_t nBytes) const;
     /**
      * Return the fee in satoshis for a size of 1000 bytes
      */
diff --git a/src/test/amount_tests.cpp b/src/test/amount_tests.cpp
index 59dab20633..fd6f88b366 100644
--- a/src/test/amount_tests.cpp
+++ b/src/test/amount_tests.cpp
@@ -27,6 +27,15 @@ BOOST_AUTO_TEST_CASE(GetFeeTest)
     BOOST_CHECK_EQUAL(feeRate.GetFee(1e3), 1e3);
     BOOST_CHECK_EQUAL(feeRate.GetFee(9e3), 9e3);
 
+    feeRate = CFeeRate(-1000);
+    // Must always just return -1 * arg
+    BOOST_CHECK_EQUAL(feeRate.GetFee(0), 0);
+    BOOST_CHECK_EQUAL(feeRate.GetFee(1), -1);
+    BOOST_CHECK_EQUAL(feeRate.GetFee(121), -121);
+    BOOST_CHECK_EQUAL(feeRate.GetFee(999), -999);
+    BOOST_CHECK_EQUAL(feeRate.GetFee(1e3), -1e3);
+    BOOST_CHECK_EQUAL(feeRate.GetFee(9e3), -9e3);
+
     feeRate = CFeeRate(123);
     // Truncates the result, if not integer
     BOOST_CHECK_EQUAL(feeRate.GetFee(0), 0);
@@ -37,6 +46,26 @@ BOOST_AUTO_TEST_CASE(GetFeeTest)
     BOOST_CHECK_EQUAL(feeRate.GetFee(999), 122);
     BOOST_CHECK_EQUAL(feeRate.GetFee(1e3), 123);
     BOOST_CHECK_EQUAL(feeRate.GetFee(9e3), 1107);
+
+    feeRate = CFeeRate(-123);
+    // Truncates the result, if not integer
+    BOOST_CHECK_EQUAL(feeRate.GetFee(0), 0);
+    BOOST_CHECK_EQUAL(feeRate.GetFee(8), -1); // Special case: returns -1 instead of 0
+    BOOST_CHECK_EQUAL(feeRate.GetFee(9), -1);
+
+    // Check full constructor
+    // 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));
+    // some more integer checks
+    BOOST_CHECK(CFeeRate(CAmount(26), 789) == CFeeRate(32));
+    BOOST_CHECK(CFeeRate(CAmount(27), 789) == CFeeRate(34));
+    // Maximum size in bytes, should not crash
+    CFeeRate(MAX_MONEY, std::numeric_limits<size_t>::max() >> 1).GetFeePerK();
 }
 
 BOOST_AUTO_TEST_SUITE_END()