Merge #6010: backport: Merge bitcoin#21848,22003

5d51855b4d Merge bitcoin/bitcoin#22003: txmempool: add thread safety annotations (MarcoFalke)
be14afca36 Merge bitcoin/bitcoin#21848: refactor: Make CFeeRate constructor architecture-independent (MarcoFalke)

Pull request description:

  backport

Top commit has no ACKs.

Tree-SHA512: eec5f474a4d182f8673509d2e163c12f78c88cc578ed60b580e9ab1c793535d1b058ad7fb8600c4f514b2351905982f94b96ad666ec47b23de849a0a99d3bd94

src/policy/feerate.cpp

@@ -7,29 +7,26 @@
 
 #include <tinyformat.h>
 
-CFeeRate::CFeeRate(const CAmount& nFeePaid, size_t nBytes_)
+CFeeRate::CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes)
 {
-    assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
+    const int64_t nSize{num_bytes};
-    int64_t nSize = int64_t(nBytes_);
 
-    if (nSize > 0)
+    if (nSize > 0) {
         nSatoshisPerK = nFeePaid * 1000 / nSize;
-    else
+    } else {
         nSatoshisPerK = 0;
     }
+}
 
-CAmount CFeeRate::GetFee(size_t nBytes_) const
+CAmount CFeeRate::GetFee(uint32_t num_bytes) const
 {
-    assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
+    const int64_t nSize{num_bytes};
-    int64_t nSize = int64_t(nBytes_);
 
     CAmount nFee = nSatoshisPerK * nSize / 1000;
 
     if (nFee == 0 && nSize != 0) {
-        if (nSatoshisPerK > 0)
+        if (nSatoshisPerK > 0) nFee = CAmount(1);
-            nFee = CAmount(1);
+        if (nSatoshisPerK < 0) nFee = CAmount(-1);
-        if (nSatoshisPerK < 0)
-            nFee = CAmount(-1);
     }
 
     return nFee;

src/policy/feerate.h

@@ -40,11 +40,11 @@ public:
         static_assert(std::is_integral<I>::value, "CFeeRate should be used without floats");
     }
     /** 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 CAmount& nFeePaid, uint32_t num_bytes);
     /**
      * Return the fee in satoshis for the given size in bytes.
      */
-    CAmount GetFee(size_t nBytes) const;
+    CAmount GetFee(uint32_t num_bytes) const;
     /**
      * Return the fee in satoshis for a size of 1000 bytes
      */

src/test/amount_tests.cpp

@@ -84,7 +84,7 @@ BOOST_AUTO_TEST_CASE(GetFeeTest)
     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();
+    CFeeRate(MAX_MONEY, std::numeric_limits<uint32_t>::max()).GetFeePerK();
 }
 
 BOOST_AUTO_TEST_CASE(BinaryOperatorTest)

src/test/fuzz/fee_rate.cpp

@@ -20,8 +20,8 @@ FUZZ_TARGET(fee_rate)
     const CFeeRate fee_rate{satoshis_per_k};
 
     (void)fee_rate.GetFeePerK();
-    const size_t bytes = fuzzed_data_provider.ConsumeIntegral<size_t>();
+    const auto bytes = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
-    if (!MultiplicationOverflow(static_cast<int64_t>(bytes), satoshis_per_k) && bytes <= static_cast<uint64_t>(std::numeric_limits<int64_t>::max())) {
+    if (!MultiplicationOverflow(int64_t{bytes}, satoshis_per_k)) {
         (void)fee_rate.GetFee(bytes);
     }
     (void)fee_rate.ToString();

src/test/fuzz/tx_pool.cpp

@@ -19,8 +19,9 @@ std::vector<COutPoint> g_outpoints_coinbase_init_mature;
 std::vector<COutPoint> g_outpoints_coinbase_init_immature;
 
 struct MockedTxPool : public CTxMemPool {
-    void RollingFeeUpdate()
+    void RollingFeeUpdate() EXCLUSIVE_LOCKS_REQUIRED(!cs)
     {
+        LOCK(cs);
         lastRollingFeeUpdate = GetTime();
         blockSinceLastRollingFeeBump = true;
     }

src/txmempool.h

@@ -472,16 +472,16 @@ class CTxMemPool
 protected:
     const int m_check_ratio; //!< Value n means that 1 times in n we check.
     std::atomic<unsigned int> nTransactionsUpdated{0}; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation
-    CBlockPolicyEstimator* minerPolicyEstimator;
+    CBlockPolicyEstimator* const minerPolicyEstimator;
     CDeterministicMNManager* m_dmnman{nullptr};
 
     uint64_t totalTxSize GUARDED_BY(cs);      //!< sum of all mempool tx' byte sizes
     CAmount m_total_fee GUARDED_BY(cs);       //!< sum of all mempool tx's fees (NOT modified fee)
     uint64_t cachedInnerUsage GUARDED_BY(cs); //!< sum of dynamic memory usage of all the map elements (NOT the maps themselves)
 
-    mutable int64_t lastRollingFeeUpdate;
+    mutable int64_t lastRollingFeeUpdate GUARDED_BY(cs);
-    mutable bool blockSinceLastRollingFeeBump;
+    mutable bool blockSinceLastRollingFeeBump GUARDED_BY(cs);
-    mutable double rollingMinimumFeeRate; //!< minimum fee to get into the pool, decreases exponentially
+    mutable double rollingMinimumFeeRate GUARDED_BY(cs); //!< minimum fee to get into the pool, decreases exponentially
     mutable Epoch m_epoch GUARDED_BY(cs);
 
     void trackPackageRemoved(const CFeeRate& rate) EXCLUSIVE_LOCKS_REQUIRED(cs);
@@ -589,7 +589,7 @@ private:
 
 public:
     indirectmap<COutPoint, const CTransaction*> mapNextTx GUARDED_BY(cs);
-    std::map<uint256, CAmount> mapDeltas;
+    std::map<uint256, CAmount> mapDeltas GUARDED_BY(cs);
 
     /** Create a new CTxMemPool.
      * Sanity checks will be off by default for performance, because otherwise