aboutsummaryrefslogtreecommitdiff
path: root/src/test
diff options
context:
space:
mode:
Diffstat (limited to 'src/test')
-rw-r--r--src/test/disconnected_transactions.cpp95
-rw-r--r--src/test/fuzz/package_eval.cpp15
-rw-r--r--src/test/fuzz/tx_pool.cpp53
-rw-r--r--src/test/util/txmempool.cpp78
-rw-r--r--src/test/util/txmempool.h10
-rw-r--r--src/test/validation_chainstatemanager_tests.cpp2
6 files changed, 241 insertions, 12 deletions
diff --git a/src/test/disconnected_transactions.cpp b/src/test/disconnected_transactions.cpp
new file mode 100644
index 0000000000..d4dc124b7b
--- /dev/null
+++ b/src/test/disconnected_transactions.cpp
@@ -0,0 +1,95 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+//
+#include <boost/test/unit_test.hpp>
+#include <core_memusage.h>
+#include <kernel/disconnected_transactions.h>
+#include <test/util/setup_common.h>
+
+BOOST_FIXTURE_TEST_SUITE(disconnected_transactions, TestChain100Setup)
+
+//! Tests that DisconnectedBlockTransactions limits its own memory properly
+BOOST_AUTO_TEST_CASE(disconnectpool_memory_limits)
+{
+ // Use the coinbase transactions from TestChain100Setup. It doesn't matter whether these
+ // transactions would realistically be in a block together, they just need distinct txids and
+ // uniform size for this test to work.
+ std::vector<CTransactionRef> block_vtx(m_coinbase_txns);
+ BOOST_CHECK_EQUAL(block_vtx.size(), 100);
+
+ // Roughly estimate sizes to sanity check that DisconnectedBlockTransactions::DynamicMemoryUsage
+ // is within an expected range.
+
+ // Overhead for the hashmap depends on number of buckets
+ std::unordered_map<uint256, CTransaction*, SaltedTxidHasher> temp_map;
+ temp_map.reserve(1);
+ const size_t MAP_1{memusage::DynamicUsage(temp_map)};
+ temp_map.reserve(100);
+ const size_t MAP_100{memusage::DynamicUsage(temp_map)};
+
+ const size_t TX_USAGE{RecursiveDynamicUsage(block_vtx.front())};
+ for (const auto& tx : block_vtx)
+ BOOST_CHECK_EQUAL(RecursiveDynamicUsage(tx), TX_USAGE);
+
+ // Our overall formula is unordered map overhead + usage per entry.
+ // Implementations may vary, but we're trying to guess the usage of data structures.
+ const size_t ENTRY_USAGE_ESTIMATE{
+ TX_USAGE
+ // list entry: 2 pointers (next pointer and prev pointer) + element itself
+ + memusage::MallocUsage((2 * sizeof(void*)) + sizeof(decltype(block_vtx)::value_type))
+ // unordered map: 1 pointer for the hashtable + key and value
+ + memusage::MallocUsage(sizeof(void*) + sizeof(decltype(temp_map)::key_type)
+ + sizeof(decltype(temp_map)::value_type))};
+
+ // DisconnectedBlockTransactions that's just big enough for 1 transaction.
+ {
+ DisconnectedBlockTransactions disconnectpool{MAP_1 + ENTRY_USAGE_ESTIMATE};
+ // Add just 2 (and not all 100) transactions to keep the unordered map's hashtable overhead
+ // to a minimum and avoid all (instead of all but 1) transactions getting evicted.
+ std::vector<CTransactionRef> two_txns({block_vtx.at(0), block_vtx.at(1)});
+ auto evicted_txns{disconnectpool.AddTransactionsFromBlock(two_txns)};
+ BOOST_CHECK(disconnectpool.DynamicMemoryUsage() <= MAP_1 + ENTRY_USAGE_ESTIMATE);
+
+ // Only 1 transaction can be kept
+ BOOST_CHECK_EQUAL(1, evicted_txns.size());
+ // Transactions are added from back to front and eviction is FIFO.
+ BOOST_CHECK_EQUAL(block_vtx.at(1), evicted_txns.front());
+
+ disconnectpool.clear();
+ }
+
+ // DisconnectedBlockTransactions with a comfortable maximum memory usage so that nothing is evicted.
+ // Record usage so we can check size limiting in the next test.
+ size_t usage_full{0};
+ {
+ const size_t USAGE_100_OVERESTIMATE{MAP_100 + ENTRY_USAGE_ESTIMATE * 100};
+ DisconnectedBlockTransactions disconnectpool{USAGE_100_OVERESTIMATE};
+ auto evicted_txns{disconnectpool.AddTransactionsFromBlock(block_vtx)};
+ BOOST_CHECK_EQUAL(evicted_txns.size(), 0);
+ BOOST_CHECK(disconnectpool.DynamicMemoryUsage() <= USAGE_100_OVERESTIMATE);
+
+ usage_full = disconnectpool.DynamicMemoryUsage();
+
+ disconnectpool.clear();
+ }
+
+ // DisconnectedBlockTransactions that's just a little too small for all of the transactions.
+ {
+ const size_t MAX_MEMUSAGE_99{usage_full - sizeof(void*)};
+ DisconnectedBlockTransactions disconnectpool{MAX_MEMUSAGE_99};
+ auto evicted_txns{disconnectpool.AddTransactionsFromBlock(block_vtx)};
+ BOOST_CHECK(disconnectpool.DynamicMemoryUsage() <= MAX_MEMUSAGE_99);
+
+ // Only 1 transaction needed to be evicted
+ BOOST_CHECK_EQUAL(1, evicted_txns.size());
+
+ // Transactions are added from back to front and eviction is FIFO.
+ // The last transaction of block_vtx should be the first to be evicted.
+ BOOST_CHECK_EQUAL(block_vtx.back(), evicted_txns.front());
+
+ disconnectpool.clear();
+ }
+}
+
+BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp
index 7220c5d997..8d316134cc 100644
--- a/src/test/fuzz/package_eval.cpp
+++ b/src/test/fuzz/package_eval.cpp
@@ -257,15 +257,6 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
const auto result_package = WITH_LOCK(::cs_main,
return ProcessNewPackage(chainstate, tx_pool, txs, /*test_accept=*/single_submit));
- // If something went wrong due to a package-specific policy, it might not return a
- // validation result for the transaction.
- if (result_package.m_state.GetResult() != PackageValidationResult::PCKG_POLICY) {
- auto it = result_package.m_tx_results.find(txs.back()->GetWitnessHash());
- Assert(it != result_package.m_tx_results.end());
- Assert(it->second.m_result_type == MempoolAcceptResult::ResultType::VALID ||
- it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID ||
- it->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY);
- }
const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, txs.back(), GetTime(), bypass_limits, /*test_accept=*/!single_submit));
const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
@@ -281,6 +272,12 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
Assert(added.size() == 1);
Assert(txs.back() == *added.begin());
}
+ } else if (result_package.m_state.GetResult() != PackageValidationResult::PCKG_POLICY) {
+ // We don't know anything about the validity since transactions were randomly generated, so
+ // just use result_package.m_state here. This makes the expect_valid check meaningless, but
+ // we can still verify that the contents of m_tx_results are consistent with m_state.
+ const bool expect_valid{result_package.m_state.IsValid()};
+ Assert(!CheckPackageMempoolAcceptResult(txs, result_package, expect_valid, nullptr));
} else {
// This is empty if it fails early checks, or "full" if transactions are looked at deeper
Assert(result_package.m_tx_results.size() == txs.size() || result_package.m_tx_results.empty());
diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp
index 5ec3e89d1e..66e537a57b 100644
--- a/src/test/fuzz/tx_pool.cpp
+++ b/src/test/fuzz/tx_pool.cpp
@@ -131,6 +131,53 @@ CTxMemPool MakeMempool(FuzzedDataProvider& fuzzed_data_provider, const NodeConte
return CTxMemPool{mempool_opts};
}
+void CheckATMPInvariants(const MempoolAcceptResult& res, bool txid_in_mempool, bool wtxid_in_mempool)
+{
+
+ switch (res.m_result_type) {
+ case MempoolAcceptResult::ResultType::VALID:
+ {
+ Assert(txid_in_mempool);
+ Assert(wtxid_in_mempool);
+ Assert(res.m_state.IsValid());
+ Assert(!res.m_state.IsInvalid());
+ Assert(res.m_replaced_transactions);
+ Assert(res.m_vsize);
+ Assert(res.m_base_fees);
+ Assert(res.m_effective_feerate);
+ Assert(res.m_wtxids_fee_calculations);
+ Assert(!res.m_other_wtxid);
+ break;
+ }
+ case MempoolAcceptResult::ResultType::INVALID:
+ {
+ // It may be already in the mempool since in ATMP cases we don't set MEMPOOL_ENTRY or DIFFERENT_WITNESS
+ Assert(!res.m_state.IsValid());
+ Assert(res.m_state.IsInvalid());
+ Assert(!res.m_replaced_transactions);
+ Assert(!res.m_vsize);
+ Assert(!res.m_base_fees);
+ // Unable or unwilling to calculate fees
+ Assert(!res.m_effective_feerate);
+ Assert(!res.m_wtxids_fee_calculations);
+ Assert(!res.m_other_wtxid);
+ break;
+ }
+ case MempoolAcceptResult::ResultType::MEMPOOL_ENTRY:
+ {
+ // ATMP never sets this; only set in package settings
+ Assert(false);
+ break;
+ }
+ case MempoolAcceptResult::ResultType::DIFFERENT_WITNESS:
+ {
+ // ATMP never sets this; only set in package settings
+ Assert(false);
+ break;
+ }
+ }
+}
+
FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
{
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
@@ -258,9 +305,11 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
SyncWithValidationInterfaceQueue();
UnregisterSharedValidationInterface(txr);
+ bool txid_in_mempool = tx_pool.exists(GenTxid::Txid(tx->GetHash()));
+ bool wtxid_in_mempool = tx_pool.exists(GenTxid::Wtxid(tx->GetWitnessHash()));
+ CheckATMPInvariants(res, txid_in_mempool, wtxid_in_mempool);
+
Assert(accepted != added.empty());
- Assert(accepted == res.m_state.IsValid());
- Assert(accepted != res.m_state.IsInvalid());
if (accepted) {
Assert(added.size() == 1); // For now, no package acceptance
Assert(tx == *added.begin());
diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp
index c945f35d79..c4fbc8dbb3 100644
--- a/src/test/util/txmempool.cpp
+++ b/src/test/util/txmempool.cpp
@@ -11,6 +11,7 @@
#include <util/check.h>
#include <util/time.h>
#include <util/translation.h>
+#include <validation.h>
using node::NodeContext;
@@ -36,3 +37,80 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransactionRef& tx) const
{
return CTxMemPoolEntry{tx, nFee, TicksSinceEpoch<std::chrono::seconds>(time), nHeight, m_sequence, spendsCoinbase, sigOpCost, lp};
}
+
+std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns,
+ const PackageMempoolAcceptResult& result,
+ bool expect_valid,
+ const CTxMemPool* mempool)
+{
+ if (expect_valid) {
+ if (result.m_state.IsInvalid()) {
+ return strprintf("Package validation unexpectedly failed: %s", result.m_state.ToString());
+ }
+ } else {
+ if (result.m_state.IsValid()) {
+ strprintf("Package validation unexpectedly succeeded. %s", result.m_state.ToString());
+ }
+ }
+ if (result.m_state.GetResult() != PackageValidationResult::PCKG_POLICY && txns.size() != result.m_tx_results.size()) {
+ strprintf("txns size %u does not match tx results size %u", txns.size(), result.m_tx_results.size());
+ }
+ for (const auto& tx : txns) {
+ const auto& wtxid = tx->GetWitnessHash();
+ if (result.m_tx_results.count(wtxid) == 0) {
+ return strprintf("result not found for tx %s", wtxid.ToString());
+ }
+
+ const auto& atmp_result = result.m_tx_results.at(wtxid);
+ const bool valid{atmp_result.m_result_type == MempoolAcceptResult::ResultType::VALID};
+ if (expect_valid && atmp_result.m_state.IsInvalid()) {
+ return strprintf("tx %s unexpectedly failed: %s", wtxid.ToString(), atmp_result.m_state.ToString());
+ }
+
+ //m_replaced_transactions should exist iff the result was VALID
+ if (atmp_result.m_replaced_transactions.has_value() != valid) {
+ return strprintf("tx %s result should %shave m_replaced_transactions",
+ wtxid.ToString(), valid ? "" : "not ");
+ }
+
+ // m_vsize and m_base_fees should exist iff the result was VALID or MEMPOOL_ENTRY
+ const bool mempool_entry{atmp_result.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY};
+ if (atmp_result.m_base_fees.has_value() != (valid || mempool_entry)) {
+ return strprintf("tx %s result should %shave m_base_fees", wtxid.ToString(), valid || mempool_entry ? "" : "not ");
+ }
+ if (atmp_result.m_vsize.has_value() != (valid || mempool_entry)) {
+ return strprintf("tx %s result should %shave m_vsize", wtxid.ToString(), valid || mempool_entry ? "" : "not ");
+ }
+
+ // m_other_wtxid should exist iff the result was DIFFERENT_WITNESS
+ const bool diff_witness{atmp_result.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS};
+ if (atmp_result.m_other_wtxid.has_value() != diff_witness) {
+ return strprintf("tx %s result should %shave m_other_wtxid", wtxid.ToString(), diff_witness ? "" : "not ");
+ }
+
+ // m_effective_feerate and m_wtxids_fee_calculations should exist iff the result was valid
+ if (atmp_result.m_effective_feerate.has_value() != valid) {
+ return strprintf("tx %s result should %shave m_effective_feerate",
+ wtxid.ToString(), valid ? "" : "not ");
+ }
+ if (atmp_result.m_wtxids_fee_calculations.has_value() != valid) {
+ return strprintf("tx %s result should %shave m_effective_feerate",
+ wtxid.ToString(), valid ? "" : "not ");
+ }
+
+ if (mempool) {
+ // The tx by txid should be in the mempool iff the result was not INVALID.
+ const bool txid_in_mempool{atmp_result.m_result_type != MempoolAcceptResult::ResultType::INVALID};
+ if (mempool->exists(GenTxid::Txid(tx->GetHash())) != txid_in_mempool) {
+ strprintf("tx %s should %sbe in mempool", wtxid.ToString(), txid_in_mempool ? "" : "not ");
+ }
+ // Additionally, if the result was DIFFERENT_WITNESS, we shouldn't be able to find the tx in mempool by wtxid.
+ if (tx->HasWitness() && atmp_result.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS) {
+ if (mempool->exists(GenTxid::Wtxid(wtxid))) {
+ strprintf("wtxid %s should not be in mempool", wtxid.ToString());
+ }
+ }
+ }
+ }
+ return std::nullopt;
+}
diff --git a/src/test/util/txmempool.h b/src/test/util/txmempool.h
index 4b0daf0d42..a866d1ce74 100644
--- a/src/test/util/txmempool.h
+++ b/src/test/util/txmempool.h
@@ -5,12 +5,14 @@
#ifndef BITCOIN_TEST_UTIL_TXMEMPOOL_H
#define BITCOIN_TEST_UTIL_TXMEMPOOL_H
+#include <policy/packages.h>
#include <txmempool.h>
#include <util/time.h>
namespace node {
struct NodeContext;
}
+struct PackageMempoolAcceptResult;
CTxMemPool::Options MemPoolOptionsForTest(const node::NodeContext& node);
@@ -36,4 +38,12 @@ struct TestMemPoolEntryHelper {
TestMemPoolEntryHelper& SigOpsCost(unsigned int _sigopsCost) { sigOpCost = _sigopsCost; return *this; }
};
+/** Check expected properties for every PackageMempoolAcceptResult, regardless of value. Returns
+ * a string if an error occurs with error populated, nullopt otherwise. If mempool is provided,
+ * checks that the expected transactions are in mempool (this should be set to nullptr for a test_accept).
+*/
+std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns,
+ const PackageMempoolAcceptResult& result,
+ bool expect_valid,
+ const CTxMemPool* mempool);
#endif // BITCOIN_TEST_UTIL_TXMEMPOOL_H
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 227d7d4633..e2541a74fd 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -579,7 +579,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
// it will initialize instead of attempting to complete validation.
//
// Note that this is not a realistic use of DisconnectTip().
- DisconnectedBlockTransactions unused_pool{MAX_DISCONNECTED_TX_POOL_SIZE * 1000};
+ DisconnectedBlockTransactions unused_pool{MAX_DISCONNECTED_TX_POOL_BYTES};
BlockValidationState unused_state;
{
LOCK2(::cs_main, bg_chainstate.MempoolMutex());
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-13 20:24:20 +0000