From fa76f0d0efccd1ea272a46060022eea3e998268e Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Wed, 14 Jun 2023 18:22:55 +0200
Subject: [PATCH] refactor: Make m_count_with_* in CTxMemPoolEntry int64_t,
 drop UBSAN supp

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.
---
 src/kernel/mempool_entry.h        | 10 +++++-----
 src/txmempool.cpp                 |  8 ++++----
 test/sanitizer_suppressions/ubsan |  1 -
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
index 969ddcd1ce277..886e1e1b3a74d 100644
--- a/src/kernel/mempool_entry.h
+++ b/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 @@ class CTxMemPoolEntry
     // 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 @@ class CTxMemPoolEntry
         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; }
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 1286eba035d55..845fbdb66e5e8 100644
--- a/src/txmempool.cpp
+++ b/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);
-    assert(int64_t(nCountWithDescendants) > 0);
+    m_count_with_descendants += modifyCount;
+    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);
-    assert(int64_t(nCountWithAncestors) > 0);
+    m_count_with_ancestors += modifyCount;
+    assert(m_count_with_ancestors > 0);
     nSigOpCostWithAncestors += modifySigOps;
     assert(int(nSigOpCostWithAncestors) >= 0);
 }
diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan
index ae70fb49d482c..8808a83e32916 100644
--- a/test/sanitizer_suppressions/ubsan
+++ b/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