Skip to content

Commit d1ae967

Browse files
committed
Merge bitcoin/bitcoin#27890: refactor: Make m_count_with_* in CTxMemPoolEntry int64_t, drop UBSAN supp
fa76f0d 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 fa76f0d ryanofsky: Code review ACK fa76f0d Tree-SHA512: a09e33a915d60c65d369d44ba1a45ce4a6a76e6dc2bea43216ba02b5eab0b74e214b2c7cc44360493f2c483d18d96e4636b7a75b23050976efc80e38de852c39
2 parents ee22ca5 + fa76f0d commit d1ae967

File tree

3 files changed

+9
-10
lines changed

3 files changed

+9
-10
lines changed

src/kernel/mempool_entry.h

+5-5
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ struct CompareIteratorByHash {
5757
* ("descendant" transactions).
5858
*
5959
* When a new entry is added to the mempool, we update the descendant state
60-
* (nCountWithDescendants, nSizeWithDescendants, and nModFeesWithDescendants) for
60+
* (m_count_with_descendants, nSizeWithDescendants, and nModFeesWithDescendants) for
6161
* all ancestors of the newly added transaction.
6262
*
6363
*/
@@ -87,13 +87,13 @@ class CTxMemPoolEntry
8787
// Information about descendants of this transaction that are in the
8888
// mempool; if we remove this transaction we must remove all of these
8989
// descendants as well.
90-
uint64_t nCountWithDescendants{1}; //!< number of descendant transactions
90+
int64_t m_count_with_descendants{1}; //!< number of descendant transactions
9191
// Using int64_t instead of int32_t to avoid signed integer overflow issues.
9292
int64_t nSizeWithDescendants; //!< ... and size
9393
CAmount nModFeesWithDescendants; //!< ... and total fees (all including us)
9494

9595
// Analogous statistics for ancestor transactions
96-
uint64_t nCountWithAncestors{1};
96+
int64_t m_count_with_ancestors{1};
9797
// Using int64_t instead of int32_t to avoid signed integer overflow issues.
9898
int64_t nSizeWithAncestors;
9999
CAmount nModFeesWithAncestors;
@@ -153,13 +153,13 @@ class CTxMemPoolEntry
153153
lockPoints = lp;
154154
}
155155

156-
uint64_t GetCountWithDescendants() const { return nCountWithDescendants; }
156+
uint64_t GetCountWithDescendants() const { return m_count_with_descendants; }
157157
int64_t GetSizeWithDescendants() const { return nSizeWithDescendants; }
158158
CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; }
159159

160160
bool GetSpendsCoinbase() const { return spendsCoinbase; }
161161

162-
uint64_t GetCountWithAncestors() const { return nCountWithAncestors; }
162+
uint64_t GetCountWithAncestors() const { return m_count_with_ancestors; }
163163
int64_t GetSizeWithAncestors() const { return nSizeWithAncestors; }
164164
CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; }
165165
int64_t GetSigOpCostWithAncestors() const { return nSigOpCostWithAncestors; }

src/txmempool.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -370,17 +370,17 @@ void CTxMemPoolEntry::UpdateDescendantState(int32_t modifySize, CAmount modifyFe
370370
nSizeWithDescendants += modifySize;
371371
assert(nSizeWithDescendants > 0);
372372
nModFeesWithDescendants = SaturatingAdd(nModFeesWithDescendants, modifyFee);
373-
nCountWithDescendants += uint64_t(modifyCount);
374-
assert(int64_t(nCountWithDescendants) > 0);
373+
m_count_with_descendants += modifyCount;
374+
assert(m_count_with_descendants > 0);
375375
}
376376

377377
void CTxMemPoolEntry::UpdateAncestorState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps)
378378
{
379379
nSizeWithAncestors += modifySize;
380380
assert(nSizeWithAncestors > 0);
381381
nModFeesWithAncestors = SaturatingAdd(nModFeesWithAncestors, modifyFee);
382-
nCountWithAncestors += uint64_t(modifyCount);
383-
assert(int64_t(nCountWithAncestors) > 0);
382+
m_count_with_ancestors += modifyCount;
383+
assert(m_count_with_ancestors > 0);
384384
nSigOpCostWithAncestors += modifySigOps;
385385
assert(int(nSigOpCostWithAncestors) >= 0);
386386
}

test/sanitizer_suppressions/ubsan

-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ unsigned-integer-overflow:hash.cpp
4646
unsigned-integer-overflow:policy/fees.cpp
4747
unsigned-integer-overflow:prevector.h
4848
unsigned-integer-overflow:script/interpreter.cpp
49-
unsigned-integer-overflow:txmempool.cpp
5049
unsigned-integer-overflow:xoroshiro128plusplus.h
5150
implicit-integer-sign-change:compat/stdin.cpp
5251
implicit-integer-sign-change:compressor.h

0 commit comments

Comments
 (0)