I understand your reasoning and the one from the linked comment, but I'm not sure it makes sense here. A lot of the comparisons we do are with other size_t's, e.g. from std::set::size(). So, we'd either have to unsafely cast them into an int64_t (probably fine, but... not ideal?) or implement a function that does it with bounds checking etc.
I'm not sure that's preferable? See e.g. the below diff, with some unsafe static_cast as well as implicit conversion (e.g. CheckPackageLimits() passing package.size() to CalculateAncestorsAndCheckLimits())
<details>
<summary>git diff</summary>
diff --git a/src/kernel/mempool_limits.h b/src/kernel/mempool_limits.h
index fd0979c6c..584af5679 100644
--- a/src/kernel/mempool_limits.h
+++ b/src/kernel/mempool_limits.h
@@ -17,20 +17,20 @@ namespace kernel {
*/
struct MemPoolLimits {
//! The maximum allowed number of transactions in a package including the entry and its ancestors.
- uint64_t ancestor_count{DEFAULT_ANCESTOR_LIMIT};
+ int64_t ancestor_count{DEFAULT_ANCESTOR_LIMIT};
//! The maximum allowed size in virtual bytes of an entry and its ancestors within a package.
- uint64_t ancestor_size_vbytes{DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * 1'000};
+ int64_t ancestor_size_vbytes{DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * 1'000};
//! The maximum allowed number of transactions in a package including the entry and its descendants.
- uint64_t descendant_count{DEFAULT_DESCENDANT_LIMIT};
+ int64_t descendant_count{DEFAULT_DESCENDANT_LIMIT};
//! The maximum allowed size in virtual bytes of an entry and its descendants within a package.
- uint64_t descendant_size_vbytes{DEFAULT_DESCENDANT_SIZE_LIMIT_KVB * 1'000};
+ int64_t descendant_size_vbytes{DEFAULT_DESCENDANT_SIZE_LIMIT_KVB * 1'000};
/**
* [@return](/bitcoin-bitcoin/contributor/return/) MemPoolLimits with all the limits set to the maximum
*/
static MemPoolLimits NoLimits()
{
- uint64_t no_limit{std::numeric_limits<uint64_t>::max()};
+ int64_t no_limit{std::numeric_limits<int64_t>::max()};
return {no_limit, no_limit, no_limit, no_limit};
}
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 3097473ac..ee5ffff7b 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -183,14 +183,14 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256>& vHashes
}
}
-bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size,
- size_t entry_count,
+bool CTxMemPool::CalculateAncestorsAndCheckLimits(int64_t entry_size,
+ int64_t entry_count,
setEntries& setAncestors,
CTxMemPoolEntry::Parents& staged_ancestors,
const Limits& limits,
std::string &errString) const
{
- size_t totalSizeWithAncestors = entry_size;
+ int64_t totalSizeWithAncestors = entry_size;
while (!staged_ancestors.empty()) {
const CTxMemPoolEntry& stage = staged_ancestors.begin()->get();
@@ -241,7 +241,7 @@ bool CTxMemPool::CheckPackageLimits(const Package& package,
std::optional<txiter> piter = GetIter(input.prevout.hash);
if (piter) {
staged_ancestors.insert(**piter);
- if (staged_ancestors.size() + package.size() > limits.ancestor_count) {
+ if (static_cast<int64_t>(staged_ancestors.size() + package.size()) > limits.ancestor_count) {
errString = strprintf("too many unconfirmed parents [limit: %u]", limits.ancestor_count);
return false;
}
@@ -277,7 +277,7 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry,
std::optional<txiter> piter = GetIter(tx.vin[i].prevout.hash);
if (piter) {
staged_ancestors.insert(**piter);
- if (staged_ancestors.size() + 1 > limits.ancestor_count) {
+ if (static_cast<int64_t>(staged_ancestors.size() + 1) > limits.ancestor_count) {
errString = strprintf("too many unconfirmed parents [limit: %u]", limits.ancestor_count);
return false;
}
</details>
