diff --git a/contrib/tracing/mempool_monitor.py b/contrib/tracing/mempool_monitor.py index 9d427d4632..afb5e60372 100755 --- a/contrib/tracing/mempool_monitor.py +++ b/contrib/tracing/mempool_monitor.py @@ -27,7 +27,7 @@ PROGRAM = """ struct added_event { u8 hash[HASH_LENGTH]; - u64 vsize; + s32 vsize; s64 fee; }; @@ -35,7 +35,7 @@ struct removed_event { u8 hash[HASH_LENGTH]; char reason[MAX_REMOVAL_REASON_LENGTH]; - u64 vsize; + s32 vsize; s64 fee; u64 entry_time; }; @@ -49,11 +49,11 @@ struct rejected_event struct replaced_event { u8 replaced_hash[HASH_LENGTH]; - u64 replaced_vsize; + s32 replaced_vsize; s64 replaced_fee; u64 replaced_entry_time; u8 replacement_hash[HASH_LENGTH]; - u64 replacement_vsize; + s32 replacement_vsize; s64 replacement_fee; }; diff --git a/doc/tracing.md b/doc/tracing.md index d26cf52fc3..0e3414205a 100644 --- a/doc/tracing.md +++ b/doc/tracing.md @@ -220,7 +220,7 @@ about the transaction. Arguments passed: 1. Transaction ID (hash) as `pointer to unsigned chars` (i.e. 32 bytes in little-endian) -2. Transaction virtual size as `uint64` +2. Transaction virtual size as `int32` 3. Transaction fee as `int64` #### Tracepoint `mempool:removed` @@ -231,7 +231,7 @@ about the transaction. Arguments passed: 1. Transaction ID (hash) as `pointer to unsigned chars` (i.e. 32 bytes in little-endian) 2. Removal reason as `pointer to C-style String` (max. length 9 characters) -3. Transaction virtual size as `uint64` +3. Transaction virtual size as `int32` 4. Transaction fee as `int64` 5. Transaction mempool entry time (epoch) as `uint64` @@ -242,11 +242,11 @@ Passes information about the replaced and replacement transactions. Arguments passed: 1. Replaced transaction ID (hash) as `pointer to unsigned chars` (i.e. 32 bytes in little-endian) -2. Replaced transaction virtual size as `uint64` +2. Replaced transaction virtual size as `int32` 3. Replaced transaction fee as `int64` 4. Replaced transaction mempool entry time (epoch) as `uint64` 5. Replacement transaction ID (hash) as `pointer to unsigned chars` (i.e. 32 bytes in little-endian) -6. Replacement transaction virtual size as `uint64` +6. Replacement transaction virtual size as `int32` 7. Replacement transaction fee as `int64` Note: In cases where a single replacement transaction replaces multiple diff --git a/src/consensus/validation.h b/src/consensus/validation.h index ad8ee676b2..d5bf08cd61 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -145,7 +145,7 @@ class BlockValidationState : public ValidationState {}; // using only serialization with and without witness data. As witness_size // is equal to total_size - stripped_size, this formula is identical to: // weight = (stripped_size * 3) + total_size. -static inline int64_t GetTransactionWeight(const CTransaction& tx) +static inline int32_t GetTransactionWeight(const CTransaction& tx) { return ::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(tx, PROTOCOL_VERSION); } diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index e1ba4296ef..969ddcd1ce 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -75,7 +75,7 @@ private: mutable Parents m_parents; mutable Children m_children; const CAmount nFee; //!< Cached to avoid expensive parent-transaction lookups - const size_t nTxWeight; //!< ... and avoid recomputing tx weight (also used for GetTxSize()) + const int32_t nTxWeight; //!< ... and avoid recomputing tx weight (also used for GetTxSize()) const size_t nUsageSize; //!< ... and total memory usage const int64_t nTime; //!< Local time when entering the mempool const unsigned int entryHeight; //!< Chain height when entering the mempool @@ -88,12 +88,14 @@ private: // mempool; if we remove this transaction we must remove all of these // descendants as well. uint64_t nCountWithDescendants{1}; //!< number of descendant transactions - uint64_t nSizeWithDescendants; //!< ... and size + // 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}; - uint64_t nSizeWithAncestors; + // Using int64_t instead of int32_t to avoid signed integer overflow issues. + int64_t nSizeWithAncestors; CAmount nModFeesWithAncestors; int64_t nSigOpCostWithAncestors; @@ -104,7 +106,7 @@ public: int64_t sigops_cost, LockPoints lp) : tx{tx}, nFee{fee}, - nTxWeight(GetTransactionWeight(*tx)), + nTxWeight{GetTransactionWeight(*tx)}, nUsageSize{RecursiveDynamicUsage(tx)}, nTime{time}, entryHeight{entry_height}, @@ -121,11 +123,11 @@ public: const CTransaction& GetTx() const { return *this->tx; } CTransactionRef GetSharedTx() const { return this->tx; } const CAmount& GetFee() const { return nFee; } - size_t GetTxSize() const + int32_t GetTxSize() const { return GetVirtualTransactionSize(nTxWeight, sigOpCost, ::nBytesPerSigOp); } - size_t GetTxWeight() const { return nTxWeight; } + int32_t GetTxWeight() const { return nTxWeight; } std::chrono::seconds GetTime() const { return std::chrono::seconds{nTime}; } unsigned int GetHeight() const { return entryHeight; } int64_t GetSigOpCost() const { return sigOpCost; } @@ -134,9 +136,9 @@ public: const LockPoints& GetLockPoints() const { return lockPoints; } // Adjusts the descendant state. - void UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount); + void UpdateDescendantState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount); // Adjusts the ancestor state - void UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps); + void UpdateAncestorState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps); // Updates the modified fees with descendants/ancestors. void UpdateModifiedFee(CAmount fee_diff) { @@ -152,13 +154,13 @@ public: } uint64_t GetCountWithDescendants() const { return nCountWithDescendants; } - uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; } + int64_t GetSizeWithDescendants() const { return nSizeWithDescendants; } CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; } bool GetSpendsCoinbase() const { return spendsCoinbase; } uint64_t GetCountWithAncestors() const { return nCountWithAncestors; } - uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; } + int64_t GetSizeWithAncestors() const { return nSizeWithAncestors; } CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; } int64_t GetSigOpCostWithAncestors() const { return nSigOpCostWithAncestors; } diff --git a/src/policy/policy.h b/src/policy/policy.h index 394fb34230..9135cae91c 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -24,7 +24,7 @@ static constexpr unsigned int DEFAULT_BLOCK_MAX_WEIGHT{MAX_BLOCK_WEIGHT - 4000}; /** Default for -blockmintxfee, which sets the minimum feerate for a transaction in blocks created by mining code **/ static constexpr unsigned int DEFAULT_BLOCK_MIN_TX_FEE{1000}; /** The maximum weight for transactions we're willing to relay/mine */ -static constexpr unsigned int MAX_STANDARD_TX_WEIGHT{400000}; +static constexpr int32_t MAX_STANDARD_TX_WEIGHT{400000}; /** The minimum non-witness size for transactions we're willing to relay/mine: one larger than 64 */ static constexpr unsigned int MIN_STANDARD_TX_NONWITNESS_SIZE{65}; /** Maximum number of signature check operations in an IsStandard() P2SH script */ diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp index 371147b0e2..b26f7dafe3 100644 --- a/src/test/miniminer_tests.cpp +++ b/src/test/miniminer_tests.cpp @@ -137,7 +137,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) std::vector all_transactions{tx1, tx2, tx3, tx4, tx5, tx6, tx7, tx8}; struct TxDimensions { - size_t vsize; CAmount mod_fee; CFeeRate feerate; + int32_t vsize; CAmount mod_fee; CFeeRate feerate; }; std::map tx_dims; for (const auto& tx : all_transactions) { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 9ce4a17c5e..cb1d8b84ca 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -75,7 +75,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendan } // descendants now contains all in-mempool descendants of updateIt. // Update and add to cached descendant map - int64_t modifySize = 0; + int32_t modifySize = 0; CAmount modifyFee = 0; int64_t modifyCount = 0; for (const CTxMemPoolEntry& descendant : descendants) { @@ -91,7 +91,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendan // Don't directly remove the transaction here -- doing so would // invalidate iterators in cachedDescendants. Mark it for removal // by inserting into descendants_to_remove. - if (descendant.GetCountWithAncestors() > uint64_t(m_limits.ancestor_count) || descendant.GetSizeWithAncestors() > uint64_t(m_limits.ancestor_size_vbytes)) { + if (descendant.GetCountWithAncestors() > uint64_t(m_limits.ancestor_count) || descendant.GetSizeWithAncestors() > m_limits.ancestor_size_vbytes) { descendants_to_remove.insert(descendant.GetTx().GetHash()); } } @@ -278,8 +278,8 @@ void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors for (const CTxMemPoolEntry& parent : parents) { UpdateChild(mapTx.iterator_to(parent), it, add); } - const int64_t updateCount = (add ? 1 : -1); - const int64_t updateSize = updateCount * it->GetTxSize(); + const int32_t updateCount = (add ? 1 : -1); + const int32_t updateSize{updateCount * it->GetTxSize()}; const CAmount updateFee = updateCount * it->GetModifiedFee(); for (txiter ancestorIt : setAncestors) { mapTx.modify(ancestorIt, [=](CTxMemPoolEntry& e) { e.UpdateDescendantState(updateSize, updateFee, updateCount); }); @@ -323,7 +323,7 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, b setEntries setDescendants; CalculateDescendants(removeIt, setDescendants); setDescendants.erase(removeIt); // don't update state for self - int64_t modifySize = -((int64_t)removeIt->GetTxSize()); + int32_t modifySize = -removeIt->GetTxSize(); CAmount modifyFee = -removeIt->GetModifiedFee(); int modifySigOps = -removeIt->GetSigOpCost(); for (txiter dit : setDescendants) { @@ -365,19 +365,19 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, b } } -void CTxMemPoolEntry::UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount) +void CTxMemPoolEntry::UpdateDescendantState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount) { nSizeWithDescendants += modifySize; - assert(int64_t(nSizeWithDescendants) > 0); + assert(nSizeWithDescendants > 0); nModFeesWithDescendants = SaturatingAdd(nModFeesWithDescendants, modifyFee); nCountWithDescendants += modifyCount; assert(int64_t(nCountWithDescendants) > 0); } -void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps) +void CTxMemPoolEntry::UpdateAncestorState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps) { nSizeWithAncestors += modifySize; - assert(int64_t(nSizeWithAncestors) > 0); + assert(nSizeWithAncestors > 0); nModFeesWithAncestors = SaturatingAdd(nModFeesWithAncestors, modifyFee); nCountWithAncestors += modifyCount; assert(int64_t(nCountWithAncestors) > 0); @@ -699,7 +699,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei // Verify ancestor state is correct. auto ancestors{AssumeCalculateMemPoolAncestors(__func__, *it, Limits::NoLimits())}; uint64_t nCountCheck = ancestors.size() + 1; - uint64_t nSizeCheck = it->GetTxSize(); + int32_t nSizeCheck = it->GetTxSize(); CAmount nFeesCheck = it->GetModifiedFee(); int64_t nSigOpCheck = it->GetSigOpCost(); @@ -720,7 +720,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei // Check children against mapNextTx CTxMemPoolEntry::Children setChildrenCheck; auto iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0)); - uint64_t child_sizes = 0; + int32_t child_sizes{0}; for (; iter != mapNextTx.end() && iter->first->hash == it->GetTx().GetHash(); ++iter) { txiter childit = mapTx.find(iter->second->GetHash()); assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions diff --git a/src/txmempool.h b/src/txmempool.h index 1ea029e949..846def02cd 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -222,7 +222,7 @@ struct TxMempoolInfo CAmount fee; /** Virtual size of the transaction. */ - size_t vsize; + int32_t vsize; /** The fee delta. */ int64_t nFeeDelta; diff --git a/test/functional/interface_usdt_mempool.py b/test/functional/interface_usdt_mempool.py index 542849d558..7f088a3ca8 100755 --- a/test/functional/interface_usdt_mempool.py +++ b/test/functional/interface_usdt_mempool.py @@ -35,7 +35,7 @@ MEMPOOL_TRACEPOINTS_PROGRAM = """ struct added_event { u8 hash[HASH_LENGTH]; - u64 vsize; + s32 vsize; s64 fee; }; @@ -43,7 +43,7 @@ struct removed_event { u8 hash[HASH_LENGTH]; char reason[MAX_REMOVAL_REASON_LENGTH]; - u64 vsize; + s32 vsize; s64 fee; u64 entry_time; }; @@ -57,11 +57,11 @@ struct rejected_event struct replaced_event { u8 replaced_hash[HASH_LENGTH]; - u64 replaced_vsize; + s32 replaced_vsize; s64 replaced_fee; u64 replaced_entry_time; u8 replacement_hash[HASH_LENGTH]; - u64 replacement_vsize; + s32 replacement_vsize; s64 replacement_fee; };