Merge bitcoin/bitcoin#24407: fees: make the class FeeFilterRounder thread-safe

8173f160e085186c9bcc7f3506205c309ee66af6 style: rename variables to match coding style (Vasil Dimov)
8b4ad203d06c5ded6ecebbd7277b29a442d88bcf fees: make FeeFilterRounder::feeset const (Vasil Dimov)
e7a5bf6be79e341e037305a4c2d8a1a510a8d709 fees: make the class FeeFilterRounder thread-safe (Vasil Dimov)

Pull request description:

  Make the class `FeeFilterRounder` thread-safe so that its methods can be called concurrently by different threads on the same object. Currently it has just one method (`round()`).

  The second commit is optional, but it improves readability, showing that the `feeset` member will never be changed, thus does not need protection from concurrent access.

ACKs for top commit:
  jonatack:
    re-ACK 8173f160e085186c9bcc7f3506205c309ee66af6
  laanwj:
    Code review ACK 8173f160e085186c9bcc7f3506205c309ee66af6
  promag:
    Code review ACK 8173f160e085186c9bcc7f3506205c309ee66af6

Tree-SHA512: 94b809997c485c0d114fa702d0406b980be8eaaebcfefa56808ed670aa943959c2f16cfd0ef72b4752fe2a409a23af1b4b7f2f236e51212957759569e3bbbefd

src/policy/fees.cpp

@@ -1011,20 +1011,36 @@ void CBlockPolicyEstimator::FlushUnconfirmed()
     LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %gs\n", num_entries, Ticks<SecondsDouble>(endclear - startclear));
 }
 
-FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee)
+static std::set<double> MakeFeeSet(const CFeeRate& min_incremental_fee,
+                                   double max_filter_fee_rate,
+                                   double fee_filter_spacing)
 {
-    CAmount minFeeLimit = std::max(CAmount(1), minIncrementalFee.GetFeePerK() / 2);
-    feeset.insert(0);
-    for (double bucketBoundary = minFeeLimit; bucketBoundary <= MAX_FILTER_FEERATE; bucketBoundary *= FEE_FILTER_SPACING) {
-        feeset.insert(bucketBoundary);
+    std::set<double> fee_set;
+
+    const CAmount min_fee_limit{std::max(CAmount(1), min_incremental_fee.GetFeePerK() / 2)};
+    fee_set.insert(0);
+    for (double bucket_boundary = min_fee_limit;
+         bucket_boundary <= max_filter_fee_rate;
+         bucket_boundary *= fee_filter_spacing) {
+
+        fee_set.insert(bucket_boundary);
     }
+
+    return fee_set;
+}
+
+FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee)
+    : m_fee_set{MakeFeeSet(minIncrementalFee, MAX_FILTER_FEERATE, FEE_FILTER_SPACING)}
+{
 }
 
 CAmount FeeFilterRounder::round(CAmount currentMinFee)
 {
-    std::set<double>::iterator it = feeset.lower_bound(currentMinFee);
-    if ((it != feeset.begin() && insecure_rand.rand32() % 3 != 0) || it == feeset.end()) {
-        it--;
+    std::set<double>::iterator it = m_fee_set.lower_bound(currentMinFee);
+    if (it == m_fee_set.end() ||
+        (it != m_fee_set.begin() &&
+         WITH_LOCK(m_insecure_rand_mutex, return insecure_rand.rand32()) % 3 != 0)) {
+        --it;
     }
     return static_cast<CAmount>(*it);
 }

src/policy/fees.h

@@ -299,14 +299,15 @@ private:
 
 public:
     /** Create new FeeFilterRounder */
-    explicit FeeFilterRounder(const CFeeRate& minIncrementalFee);
+    explicit FeeFilterRounder(const CFeeRate& min_incremental_fee);
 
-    /** Quantize a minimum fee for privacy purpose before broadcast. Not thread-safe due to use of FastRandomContext */
+    /** Quantize a minimum fee for privacy purpose before broadcast. */
     CAmount round(CAmount currentMinFee);
 
 private:
-    std::set<double> feeset;
-    FastRandomContext insecure_rand;
+    const std::set<double> m_fee_set;
+    Mutex m_insecure_rand_mutex;
+    FastRandomContext insecure_rand GUARDED_BY(m_insecure_rand_mutex);
 };
 
 #endif // BITCOIN_POLICY_FEES_H