Skip to content

Commit

Permalink
[fuzz] v3 transactions and sigop-adjusted vsize
Browse files Browse the repository at this point in the history
Ensure we are checking sigop-adjusted virtual size by creating setups
and packages where sigop cost is larger than bip141 vsize.

Co-authored-by: Gregory Sanders <gsanders87@gmail.com>
  • Loading branch information
glozow and instagibbs committed Feb 8, 2024
1 parent 1fd16b5 commit e643ea7
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 12 deletions.
35 changes: 24 additions & 11 deletions src/test/fuzz/package_eval.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <node/context.h>
#include <node/mempool_args.h>
#include <node/miner.h>
#include <policy/v3_policy.h>
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h>
Expand Down Expand Up @@ -119,7 +120,8 @@ CTxMemPool MakeMempool(FuzzedDataProvider& fuzzed_data_provider, const NodeConte
mempool_opts.limits.descendant_size_vbytes = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 202) * 1'000;
mempool_opts.max_size_bytes = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 200) * 1'000'000;
mempool_opts.expiry = std::chrono::hours{fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 999)};
nBytesPerSigOp = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(1, 999);
// Only interested in 2 cases: sigop cost 0 or when single legacy sigop cost is >> 1KvB
nBytesPerSigOp = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 1) * 10'000;

mempool_opts.check_ratio = 1;
mempool_opts.require_standard = fuzzed_data_provider.ConsumeBool();
Expand Down Expand Up @@ -171,11 +173,11 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
// Create transaction to add to the mempool
const CTransactionRef tx = [&] {
CMutableTransaction tx_mut;
tx_mut.nVersion = CTransaction::CURRENT_VERSION;
tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ? 3 : CTransaction::CURRENT_VERSION;
tx_mut.nLockTime = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<uint32_t>();
// Last tx will sweep all outpoints in package
const auto num_in = last_tx ? package_outpoints.size() : fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size());
const auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size() * 2);
auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size() * 2);

auto& outpoints = last_tx ? package_outpoints : mempool_outpoints;

Expand Down Expand Up @@ -211,17 +213,24 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
tx_mut.vin.push_back(tx_mut.vin.back());
}

// Refer to a non-existant input
// Refer to a non-existent input
if (fuzzed_data_provider.ConsumeBool()) {
tx_mut.vin.emplace_back();
}

// Make a p2pk output to make sigops adjusted vsize to violate v3, potentially, which is never spent
if (last_tx && amount_in > 1000 && fuzzed_data_provider.ConsumeBool()) {
tx_mut.vout.emplace_back(1000, CScript() << std::vector<unsigned char>(33, 0x02) << OP_CHECKSIG);
// Don't add any other outputs.
num_out = 1;
amount_in -= 1000;
}

const auto amount_fee = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(0, amount_in);
const auto amount_out = (amount_in - amount_fee) / num_out;
for (int i = 0; i < num_out; ++i) {
tx_mut.vout.emplace_back(amount_out, P2WSH_EMPTY);
}
// TODO vary transaction sizes to catch size-related issues
auto tx = MakeTransactionRef(tx_mut);
// Restore previously removed outpoints, except in-package outpoints
if (!last_tx) {
Expand Down Expand Up @@ -261,7 +270,6 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
std::set<CTransactionRef> added;
auto txr = std::make_shared<TransactionsDelta>(added);
RegisterSharedValidationInterface(txr);
const bool bypass_limits = fuzzed_data_provider.ConsumeBool();

// When there are multiple transactions in the package, we call ProcessNewPackage(txs, test_accept=false)
// and AcceptToMemoryPool(txs.back(), test_accept=true). When there is only 1 transaction, we might flip it
Expand All @@ -271,17 +279,20 @@ 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));

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;
// Always set bypass_limits to false because it is not supported in ProcessNewPackage and
// can be a source of divergence.
const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, txs.back(), GetTime(),
/*bypass_limits=*/false, /*test_accept=*/!single_submit));
const bool passed = res.m_result_type == MempoolAcceptResult::ResultType::VALID;

SyncWithValidationInterfaceQueue();
UnregisterSharedValidationInterface(txr);

// There is only 1 transaction in the package. We did a test-package-accept and a ATMP
if (single_submit) {
Assert(accepted != added.empty());
Assert(accepted == res.m_state.IsValid());
if (accepted) {
Assert(passed != added.empty());
Assert(passed == res.m_state.IsValid());
if (passed) {
Assert(added.size() == 1);
Assert(txs.back() == *added.begin());
}
Expand All @@ -295,6 +306,8 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
// 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());
}

CheckMempoolV3Invariants(tx_pool);
}

UnregisterSharedValidationInterface(outpoints_updater);
Expand Down
7 changes: 6 additions & 1 deletion src/test/fuzz/tx_pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <node/context.h>
#include <node/mempool_args.h>
#include <node/miner.h>
#include <policy/v3_policy.h>
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h>
Expand Down Expand Up @@ -229,7 +230,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
// Create transaction to add to the mempool
const CTransactionRef tx = [&] {
CMutableTransaction tx_mut;
tx_mut.nVersion = CTransaction::CURRENT_VERSION;
tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ? 3 : CTransaction::CURRENT_VERSION;
tx_mut.nLockTime = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<uint32_t>();
const auto num_in = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, outpoints_rbf.size());
const auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, outpoints_rbf.size() * 2);
Expand Down Expand Up @@ -315,6 +316,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
if (accepted) {
Assert(added.size() == 1); // For now, no package acceptance
Assert(tx == *added.begin());
CheckMempoolV3Invariants(tx_pool);
} else {
// Do not consider rejected transaction removed
removed.erase(tx);
Expand Down Expand Up @@ -407,6 +409,9 @@ FUZZ_TARGET(tx_pool, .init = initialize_tx_pool)
const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
if (accepted) {
txids.push_back(tx->GetHash());
// Only check fees if accepted and not bypass_limits, otherwise it's not guaranteed that
// trimming has happened for this tx and previous iterations.
CheckMempoolV3Invariants(tx_pool);
}
}
Finish(fuzzed_data_provider, tx_pool, chainstate);
Expand Down
26 changes: 26 additions & 0 deletions src/test/util/txmempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <chainparams.h>
#include <node/context.h>
#include <node/mempool_args.h>
#include <policy/v3_policy.h>
#include <txmempool.h>
#include <util/check.h>
#include <util/time.h>
Expand Down Expand Up @@ -116,3 +117,28 @@ std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns,
}
return std::nullopt;
}

void CheckMempoolV3Invariants(const CTxMemPool& tx_pool)
{
LOCK(tx_pool.cs);
for (const auto& tx_info : tx_pool.infoAll()) {
const auto& entry = *Assert(tx_pool.GetEntry(tx_info.tx->GetHash()));
if (tx_info.tx->nVersion == 3) {
// Check that special v3 ancestor/descendant limits and rules are always respected
Assert(entry.GetCountWithDescendants() <= V3_DESCENDANT_LIMIT);
Assert(entry.GetCountWithAncestors() <= V3_ANCESTOR_LIMIT);
// If this transaction has at least 1 ancestor, it's a "child" and has restricted weight.
if (entry.GetCountWithAncestors() > 1) {
Assert(entry.GetTxSize() <= V3_CHILD_MAX_VSIZE);
// All v3 transactions must only have v3 unconfirmed parents.
const auto& parents = entry.GetMemPoolParentsConst();
Assert(parents.begin()->get().GetSharedTx()->nVersion == 3);
}
} else if (entry.GetCountWithAncestors() > 1) {
// All non-v3 transactions must only have non-v3 unconfirmed parents.
for (const auto& parent : entry.GetMemPoolParentsConst()) {
Assert(parent.get().GetSharedTx()->nVersion != 3);
}
}
}
}
10 changes: 10 additions & 0 deletions src/test/util/txmempool.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,14 @@ std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns,
const PackageMempoolAcceptResult& result,
bool expect_valid,
const CTxMemPool* mempool);

/** For every transaction in tx_pool, check v3 invariants:
* - a v3 tx's ancestor count must be within V3_ANCESTOR_LIMIT
* - a v3 tx's descendant count must be within V3_DESCENDANT_LIMIT
* - if a v3 tx has ancestors, its sigop-adjusted vsize must be within V3_CHILD_MAX_VSIZE
* - any non-v3 tx must only have non-v3 parents
* - any v3 tx must only have v3 parents
* */
void CheckMempoolV3Invariants(const CTxMemPool& tx_pool);

#endif // BITCOIN_TEST_UTIL_TXMEMPOOL_H

0 comments on commit e643ea7

Please sign in to comment.