Merge bitcoin/bitcoin#27890: refactor: Make m_count_with_* in CTxMemPoolEntry int64_t, drop UBSAN supp

fa76f0d0ef refactor: Make m_count_with_* in CTxMemPoolEntry int64_t, drop UBSAN supp (MarcoFalke)

Pull request description:

  This is a refactor as long as no signed integer overflow appears. In normal operation and absent bugs, signed integer overflow should never happen in the touched code paths.

  The main benefit of this refactor is to drop the file-wide ubsan suppression `unsigned-integer-overflow:txmempool.cpp`.

  For now, this only changes the internal private representation and the publicly returned type remains `uint64_t`.

ACKs for top commit:
  glozow:
    ACK fa76f0d0ef
  ryanofsky:
    Code review ACK fa76f0d0ef

Tree-SHA512: a09e33a915d60c65d369d44ba1a45ce4a6a76e6dc2bea43216ba02b5eab0b74e214b2c7cc44360493f2c483d18d96e4636b7a75b23050976efc80e38de852c39

src/kernel/mempool_entry.h

@@ -57,7 +57,7 @@ struct CompareIteratorByHash {
  * ("descendant" transactions).
  *
  * When a new entry is added to the mempool, we update the descendant state
- * (nCountWithDescendants, nSizeWithDescendants, and nModFeesWithDescendants) for
+ * (m_count_with_descendants, nSizeWithDescendants, and nModFeesWithDescendants) for
  * all ancestors of the newly added transaction.
  *
  */
@@ -87,13 +87,13 @@ private:
     // Information about descendants of this transaction that are in the
     // mempool; if we remove this transaction we must remove all of these
     // descendants as well.
-    uint64_t nCountWithDescendants{1}; //!< number of descendant transactions
+    int64_t m_count_with_descendants{1}; //!< number of descendant transactions
     // Using int64_t instead of int32_t to avoid signed integer overflow issues.
     int64_t nSizeWithDescendants;      //!< ... and size
     CAmount nModFeesWithDescendants;   //!< ... and total fees (all including us)
 
     // Analogous statistics for ancestor transactions
-    uint64_t nCountWithAncestors{1};
+    int64_t m_count_with_ancestors{1};
     // Using int64_t instead of int32_t to avoid signed integer overflow issues.
     int64_t nSizeWithAncestors;
     CAmount nModFeesWithAncestors;
@@ -153,13 +153,13 @@ public:
         lockPoints = lp;
     }
 
-    uint64_t GetCountWithDescendants() const { return nCountWithDescendants; }
+    uint64_t GetCountWithDescendants() const { return m_count_with_descendants; }
     int64_t GetSizeWithDescendants() const { return nSizeWithDescendants; }
     CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; }
 
     bool GetSpendsCoinbase() const { return spendsCoinbase; }
 
-    uint64_t GetCountWithAncestors() const { return nCountWithAncestors; }
+    uint64_t GetCountWithAncestors() const { return m_count_with_ancestors; }
     int64_t GetSizeWithAncestors() const { return nSizeWithAncestors; }
     CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; }
     int64_t GetSigOpCostWithAncestors() const { return nSigOpCostWithAncestors; }

src/txmempool.cpp

@@ -370,8 +370,8 @@ void CTxMemPoolEntry::UpdateDescendantState(int32_t modifySize, CAmount modifyFe
     nSizeWithDescendants += modifySize;
     assert(nSizeWithDescendants > 0);
     nModFeesWithDescendants = SaturatingAdd(nModFeesWithDescendants, modifyFee);
-    nCountWithDescendants += uint64_t(modifyCount);
+    m_count_with_descendants += modifyCount;
-    assert(int64_t(nCountWithDescendants) > 0);
+    assert(m_count_with_descendants > 0);
 }
 
 void CTxMemPoolEntry::UpdateAncestorState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps)
@@ -379,8 +379,8 @@ void CTxMemPoolEntry::UpdateAncestorState(int32_t modifySize, CAmount modifyFee,
     nSizeWithAncestors += modifySize;
     assert(nSizeWithAncestors > 0);
     nModFeesWithAncestors = SaturatingAdd(nModFeesWithAncestors, modifyFee);
-    nCountWithAncestors += uint64_t(modifyCount);
+    m_count_with_ancestors += modifyCount;
-    assert(int64_t(nCountWithAncestors) > 0);
+    assert(m_count_with_ancestors > 0);
     nSigOpCostWithAncestors += modifySigOps;
     assert(int(nSigOpCostWithAncestors) >= 0);
 }

test/sanitizer_suppressions/ubsan

@@ -46,7 +46,6 @@ unsigned-integer-overflow:hash.cpp
 unsigned-integer-overflow:policy/fees.cpp
 unsigned-integer-overflow:prevector.h
 unsigned-integer-overflow:script/interpreter.cpp
-unsigned-integer-overflow:txmempool.cpp
 unsigned-integer-overflow:xoroshiro128plusplus.h
 implicit-integer-sign-change:compat/stdin.cpp
 implicit-integer-sign-change:compressor.h