Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#27890: refactor: Make m_count_with_* in CTxMemP…
Browse files Browse the repository at this point in the history
…oolEntry 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
  • Loading branch information
glozow committed Jun 20, 2023
2 parents ee22ca5 + fa76f0d commit d1ae967
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 10 deletions.
10 changes: 5 additions & 5 deletions src/kernel/mempool_entry.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
*/
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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; }
Expand Down
8 changes: 4 additions & 4 deletions src/txmempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,17 +370,17 @@ 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)
{
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);
}
Expand Down
1 change: 0 additions & 1 deletion test/sanitizer_suppressions/ubsan
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d1ae967

Please sign in to comment.