diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 90a897eabb..47eaabe5c2 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -8,6 +8,7 @@ #include #include #include // For CTransactionRef +#include #include // ELEMENTS For MempoolAcceptResult #include @@ -262,7 +263,7 @@ public: virtual void getPackageLimits(unsigned int& limit_ancestor_count, unsigned int& limit_descendant_count) = 0; //! Check if transaction will pass the mempool's chain limits. - virtual bool checkChainLimits(const CTransactionRef& tx) = 0; + virtual util::Result checkChainLimits(const CTransactionRef& tx) = 0; //! Estimate smart fee. virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc = nullptr) = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index f523243e31..8b42fbbbc3 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -703,15 +704,14 @@ public: limit_ancestor_count = limits.ancestor_count; limit_descendant_count = limits.descendant_count; } - bool checkChainLimits(const CTransactionRef& tx) override + util::Result checkChainLimits(const CTransactionRef& tx) override { - if (!m_node.mempool) return true; + if (!m_node.mempool) return {}; LockPoints lp; std::set> setPeginsSpent; CTxMemPoolEntry entry(tx, 0, 0, 0, 0, false, 0, lp, setPeginsSpent); LOCK(m_node.mempool->cs); - std::string err_string; - return m_node.mempool->CheckPackageLimits({tx}, entry.GetTxSize(), err_string); + return m_node.mempool->CheckPackageLimits({tx}, entry.GetTxSize()); } CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 403e9581cf..a4585c17d9 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -200,25 +200,20 @@ util::Result CTxMemPool::CalculateAncestorsAndCheckLimit return ancestors; } -bool CTxMemPool::CheckPackageLimits(const Package& package, - const int64_t total_vsize, - std::string &errString) const +util::Result CTxMemPool::CheckPackageLimits(const Package& package, + const int64_t total_vsize) const { size_t pack_count = package.size(); // Package itself is busting mempool limits; should be rejected even if no staged_ancestors exist if (pack_count > static_cast(m_limits.ancestor_count)) { - errString = strprintf("package count %u exceeds ancestor count limit [limit: %u]", pack_count, m_limits.ancestor_count); - return false; + return util::Error{Untranslated(strprintf("package count %u exceeds ancestor count limit [limit: %u]", pack_count, m_limits.ancestor_count))}; } else if (pack_count > static_cast(m_limits.descendant_count)) { - errString = strprintf("package count %u exceeds descendant count limit [limit: %u]", pack_count, m_limits.descendant_count); - return false; + return util::Error{Untranslated(strprintf("package count %u exceeds descendant count limit [limit: %u]", pack_count, m_limits.descendant_count))}; } else if (total_vsize > m_limits.ancestor_size_vbytes) { - errString = strprintf("package size %u exceeds ancestor size limit [limit: %u]", total_vsize, m_limits.ancestor_size_vbytes); - return false; + return util::Error{Untranslated(strprintf("package size %u exceeds ancestor size limit [limit: %u]", total_vsize, m_limits.ancestor_size_vbytes))}; } else if (total_vsize > m_limits.descendant_size_vbytes) { - errString = strprintf("package size %u exceeds descendant size limit [limit: %u]", total_vsize, m_limits.descendant_size_vbytes); - return false; + return util::Error{Untranslated(strprintf("package size %u exceeds descendant size limit [limit: %u]", total_vsize, m_limits.descendant_size_vbytes))}; } CTxMemPoolEntry::Parents staged_ancestors; @@ -228,8 +223,7 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, if (piter) { staged_ancestors.insert(**piter); if (staged_ancestors.size() + package.size() > static_cast(m_limits.ancestor_count)) { - errString = strprintf("too many unconfirmed parents [limit: %u]", m_limits.ancestor_count); - return false; + return util::Error{Untranslated(strprintf("too many unconfirmed parents [limit: %u]", m_limits.ancestor_count))}; } } } @@ -240,8 +234,8 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, const auto ancestors{CalculateAncestorsAndCheckLimits(total_vsize, package.size(), staged_ancestors, m_limits)}; // It's possible to overestimate the ancestor/descendant totals. - if (!ancestors.has_value()) errString = "possibly " + util::ErrorString(ancestors).original; - return ancestors.has_value(); + if (!ancestors.has_value()) return util::Error{Untranslated("possibly " + util::ErrorString(ancestors).original)}; + return {}; } util::Result CTxMemPool::CalculateMemPoolAncestors( diff --git a/src/txmempool.h b/src/txmempool.h index 91471aa2ba..bbf59bf667 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -662,11 +662,9 @@ public: * 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[out] errString Populated with error reason if a limit is hit. */ - bool CheckPackageLimits(const Package& package, - int64_t total_vsize, - std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); + util::Result CheckPackageLimits(const Package& package, + int64_t total_vsize) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Populate setDescendants with all in-mempool descendants of hash. * Assumes that setDescendants includes all in-mempool descendants of anything diff --git a/src/validation.cpp b/src/validation.cpp index 3131e45664..ee43e88aa2 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -55,6 +55,7 @@ #include #include #include +#include #include #include #include @@ -1117,10 +1118,10 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector& txn assert(std::all_of(txns.cbegin(), txns.cend(), [this](const auto& tx) { return !m_pool.exists(GenTxid::Txid(tx->GetHash()));})); - std::string err_string; - if (!m_pool.CheckPackageLimits(txns, total_vsize, err_string)) { + auto result = m_pool.CheckPackageLimits(txns, total_vsize); + if (!result) { // This is a package-wide error, separate from an individual transaction error. - return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", err_string); + return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", util::ErrorString(result).original); } return true; } diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 153626588e..d44c917b2f 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -2038,8 +2038,9 @@ static util::Result CreateTransactionInternal( if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits - if (!wallet.chain().checkChainLimits(tx)) { - return util::Error{_("Transaction has too long of a mempool chain")}; + auto result = wallet.chain().checkChainLimits(tx); + if (!result) { + return util::Error{util::ErrorString(result)}; } } diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 7a69e59d5f..c7bbbd6c57 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -650,7 +650,7 @@ class WalletTest(BitcoinTestFramework): node0_balance = self.nodes[0].getbalance()['bitcoin'] # With walletrejectlongchains we will not create the tx and store it in our wallet. - assert_raises_rpc_error(-6, "Transaction has too long of a mempool chain", self.nodes[0].sendtoaddress, sending_addr, node0_balance - Decimal('0.01')) + assert_raises_rpc_error(-6, f"too many unconfirmed ancestors [limit: {chainlimit * 2}]", self.nodes[0].sendtoaddress, sending_addr, node0_balance - Decimal('0.01')) # Verify nothing new in wallet assert_equal(total_txs, len(self.nodes[0].listtransactions("*", 99999)))