From 1d8b2d966c00fbcaa8effcc260780bd9ff1dffcb Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 26 Aug 2023 00:09:16 +0000 Subject: [PATCH] Bugfix: Pass correct virtual size to CheckPackageLimits Github-Pull: #28471 Rebased-From: bc013fe8e3d0bae2ab766a8248219aa3c9e344df --- src/txmempool.cpp | 5 ++--- src/txmempool.h | 2 ++ src/validation.cpp | 6 ++++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 032dfee3ea..1a6883adea 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -195,13 +195,12 @@ util::Result CTxMemPool::CalculateAncestorsAndCheckLimit } bool CTxMemPool::CheckPackageLimits(const Package& package, + const int64_t total_vsize, const Limits& limits, std::string &errString) const { CTxMemPoolEntry::Parents staged_ancestors; - size_t total_size = 0; for (const auto& tx : package) { - total_size += GetVirtualTransactionSize(*tx); for (const auto& input : tx->vin) { std::optional piter = GetIter(input.prevout.hash); if (piter) { @@ -216,7 +215,7 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, // When multiple transactions are passed in, the ancestors and descendants of all transactions // considered together must be within limits even if they are not interdependent. This may be // stricter than the limits for each individual transaction. - const auto ancestors{CalculateAncestorsAndCheckLimits(total_size, package.size(), + const auto ancestors{CalculateAncestorsAndCheckLimits(total_vsize, package.size(), staged_ancestors, limits)}; // It's possible to overestimate the ancestor/descendant totals. if (!ancestors.has_value()) errString = "possibly " + util::ErrorString(ancestors).original; diff --git a/src/txmempool.h b/src/txmempool.h index 2c3cb7e9db..d545c71393 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -594,10 +594,12 @@ public: * @param[in] package Transaction package being evaluated for acceptance * to mempool. The transactions need not be direct * ancestors/descendants of each other. + * @param[in] total_vsize Sum of virtual sizes for all transactions in package. * @param[in] limits Maximum number and size of ancestors and descendants * @param[out] errString Populated with error reason if a limit is hit. */ bool CheckPackageLimits(const Package& package, + int64_t total_vsize, const Limits& limits, std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/validation.cpp b/src/validation.cpp index 1fd8f0e326..12f2611c30 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -623,6 +623,7 @@ private: // Enforce package mempool ancestor/descendant limits (distinct from individual // ancestor/descendant limits done in PreChecks). bool PackageMempoolChecks(const std::vector& txns, + int64_t total_vsize, PackageValidationState& package_state) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); // Run the script checks using our policy flags. As this can be slow, we should @@ -978,6 +979,7 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws) } bool MemPoolAccept::PackageMempoolChecks(const std::vector& txns, + const int64_t total_vsize, PackageValidationState& package_state) { AssertLockHeld(cs_main); @@ -988,7 +990,7 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector& txn { return !m_pool.exists(GenTxid::Txid(tx->GetHash()));})); std::string err_string; - if (!m_pool.CheckPackageLimits(txns, m_limits, err_string)) { + if (!m_pool.CheckPackageLimits(txns, total_vsize, m_limits, err_string)) { // This is a package-wide error, separate from an individual transaction error. return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", err_string); } @@ -1281,7 +1283,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: // because it's unnecessary. Also, CPFP carve out can increase the limit for individual // transactions, but this exemption is not extended to packages in CheckPackageLimits(). std::string err_string; - if (txns.size() > 1 && !PackageMempoolChecks(txns, package_state)) { + if (txns.size() > 1 && !PackageMempoolChecks(txns, m_total_vsize, package_state)) { return PackageMempoolAcceptResult(package_state, std::move(results)); }