aboutsummaryrefslogtreecommitdiff
path: root/src/test
diff options
context:
space:
mode:
authorglozow <gloriajzhao@gmail.com>2023-11-03 13:48:24 +0000
committerglozow <gloriajzhao@gmail.com>2023-11-03 13:51:12 +0000
commitf23ac10ca5a322e87664b58233dccc4cb74c0570 (patch)
tree60fd8632ef99a5f50663f70ac89a9087229b85e6 /src/test
parent9b68c9b85efebfa23daec6471b87e9cbb514a006 (diff)
parentfcb3069fa307942cf7f3edabcda1be96d615c91f (diff)
Merge bitcoin/bitcoin#28764: Fuzz: Check individual and package transaction invariants
fcb3069fa307942cf7f3edabcda1be96d615c91f Use CheckPackageMempoolAcceptResult for package evaluation fuzzing (Greg Sanders) 34088d6c9ed4ed99bb6b7fc83795da01ec9f3c97 [test util] CheckPackageMempoolAcceptResult for sanity-checking results (glozow) 651fa404e454e31f8e9d830aa292eb3b456b54fb fuzz: tx_pool checks ATMP result invariants (Greg Sanders) Pull request description: Poached from https://github.com/bitcoin/bitcoin/pull/26711 since that PR is being split apart, and modified to match current behavior. ACKs for top commit: glozow: reACK fcb3069fa307942cf7f3edabcda1be96d615c91f, only whitespace changes dergoegge: ACK fcb3069fa307942cf7f3edabcda1be96d615c91f Tree-SHA512: abd687e526d8dfc8d65b3a873ece8ca35fdcbd6b0f7b93da6a723ef4e47cf85612de819e6f2b8631bdf897e1aba27cdd86f89b7bd85fc3356e74be275dcdf8cc
Diffstat (limited to 'src/test')
-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
4 files changed, 145 insertions, 11 deletions
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
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-18 08:25:43 +0000