Merge bitcoin/bitcoin#30110: refactor: TxDownloadManager + fuzzing

0f4bc63585 [fuzz] txdownloadman and txdownload_impl (glozow)
699643f23a [unit test] MempoolRejectedTx (glozow)
fa584cbe72 [p2p] add TxDownloadOptions bool to make TxRequestTracker deterministic (glozow)
f803c8ce8d [p2p] filter 1p1c for child txid in recent rejects (glozow)
5269d57e6d [p2p] don't process orphan if in recent rejects (glozow)
2266eba43a [p2p] don't find 1p1cs for reconsiderable txns that are AlreadyHaveTx (glozow)
fa7027d0fc [refactor] add CheckIsEmpty and GetOrphanTransactions, remove access to TxDownloadMan internals (glozow)
969b07237b [refactor] wrap {Have,Get}TxToReconsider in txdownload (glozow)
f150fb94e7 [refactor] make AlreadyHaveTx and Find1P1CPackage private to TxDownloadImpl (glozow)
1e08195135 [refactor] move new tx logic to txdownload (glozow)
257568eab5 [refactor] move invalid package processing to TxDownload (glozow)
c4ce0c1218 [refactor] move invalid tx processing to TxDownload (glozow)
c6b21749ca [refactor] move valid tx processing to TxDownload (glozow)
a8cf3b6e84 [refactor] move Find1P1CPackage to txdownload (glozow)
f497414ce7 [refactor] put peerman tasks at the end of ProcessInvalidTx (glozow)
6797bc42a7 [p2p] restrict RecursiveDynamicUsage of orphans added to vExtraTxnForCompact (glozow)
798cc8f5aa [refactor] move Find1P1CPackage into ProcessInvalidTx (glozow)
416fbc952b [refactor] move new orphan handling to ProcessInvalidTx (glozow)
c8e67b9169 [refactor] move ProcessInvalidTx and ProcessValidTx definitions down (glozow)
3a41926d1b [refactor] move notfound processing to txdownload (glozow)
042a97ce7f [refactor] move tx inv/getdata handling to txdownload (glozow)
58e09f244b [p2p] don't log tx invs when in IBD (glozow)
288865338f [refactor] rename maybe_add_extra_compact_tx to first_time_failure (glozow)
f48d36cd97 [refactor] move peer (dis)connection logic to TxDownload (glozow)
f61d9e4b4b [refactor] move AlreadyHaveTx to TxDownload (glozow)
84e4ef843d [txdownload] add read-only reference to mempool (glozow)
af918349de [refactor] move ValidationInterface functions to TxDownloadManager (glozow)
f6c860efb1 [doc] fix typo in m_lazy_recent_confirmed_transactions doc (glozow)
5f9004e155 [refactor] add TxDownloadManager wrapping TxOrphanage, TxRequestTracker, and bloom filters (glozow)

Pull request description:

  Part of #27463.

  This PR does 3 things:

  (1) It modularizes transaction download logic into a `TxDownloadManager`. Transaction download logic refers to the process of deciding what transactions to request, download, and validate.[1] There should be no behavior changes. Using `--color_moved=dimmed_zebra -w` may help.
  (2) It adds unit and fuzz (🪄) testing for transaction download.
  (3) It makes a few small behavioral changes:
  - Stop (debug-only) logging tx invs during IBD
  - Just like all other transactions, require orphans have RecursiveDynamicUsage < 100k before adding to vExtraTxnForCompact
  - Don't return a 1p1c that contains a parent or child in recent rejects. Don't process any orphan already in recent rejects. These cases should not happen in actual node operation; it's just to allow tighter sanity checks during fuzzing.

  There are several benefits to this interface, such as:
  - Unit test coverage and fuzzing for logic that currently isn't feasible to test as thoroughly (without lots of overhead) and/or currently only lightly tested through `assert_debug_log` (not good) in functional tests.
  - When we add more functionality (e.g. package relay messages, more robust orphan handling), the vast majority of it will be within `TxDownloadManager` instead of `PeerManager`, making it easier to review and test. See #28031 for what this looks like.
  - `PeerManager` will no longer know anything about / have access to `TxOrphanage`, `TxRequestTracker` or the rejection caches. Its primary interface with `TxDownloadManager` would be much simpler:
      - Passing on  `ValidationInterface` callbacks
      - Telling `txdownloadman` when a peer {connects, disconnects}
      - Telling `txdownloadman`when a {transaction, package} is {accepted, rejected} from mempool
      - Telling `txdownloadman` when invs, notfounds, and txs are received.
      - Getting instructions on what to download.
      - Getting instructions on what {transactions, packages, orphans} to validate.
      - Get whether a peer `HaveMoreWork` for the `ProessMessages` loop
  - (todo) Thread-safety can be handled internally.

  [1]: This module is concerned with tx *download*, not upload. It excludes transaction announcements/gossip which happens after we download/accept a transaction. Txreconciliation (erlay) is excluded from this module, as it only relates to deciding which `inv`s to send or helping the other peer decide which `inv`s to send. It is independent from this logic.

ACKs for top commit:
  achow101:
    light ACK 0f4bc63585
  theStack:
    ACK 0f4bc63585
  instagibbs:
    reACK 0f4bc63585
  naumenkogs:
    ACK 0f4bc63585

Tree-SHA512: 84ab8ef8a0fc705eb829d7f7d6885f28944aaa42b03172f256a42605677b3e783919bb900d4e3b8589f85a0c387dfbd972bcd61d252d44a88c6aaa90e4bf920f

src/test/fuzz/CMakeLists.txt

@@ -117,6 +117,7 @@ add_executable(fuzz
   timeoffsets.cpp
   torcontrol.cpp
   transaction.cpp
+  txdownloadman.cpp
   tx_in.cpp
   tx_out.cpp
   tx_pool.cpp

src/test/fuzz/txdownloadman.cpp

@@ -0,0 +1,445 @@
+// 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 <consensus/validation.h>
+#include <node/context.h>
+#include <node/mempool_args.h>
+#include <node/miner.h>
+#include <node/txdownloadman.h>
+#include <node/txdownloadman_impl.h>
+#include <test/fuzz/FuzzedDataProvider.h>
+#include <test/fuzz/fuzz.h>
+#include <test/fuzz/util.h>
+#include <test/fuzz/util/mempool.h>
+#include <test/util/mining.h>
+#include <test/util/script.h>
+#include <test/util/setup_common.h>
+#include <test/util/txmempool.h>
+#include <util/hasher.h>
+#include <util/rbf.h>
+#include <txmempool.h>
+#include <validation.h>
+#include <validationinterface.h>
+
+namespace {
+
+const TestingSetup* g_setup;
+
+constexpr size_t NUM_COINS{50};
+COutPoint COINS[NUM_COINS];
+
+static TxValidationResult TESTED_TX_RESULTS[] = {
+    // Skip TX_RESULT_UNSET
+    TxValidationResult::TX_CONSENSUS,
+    TxValidationResult::TX_RECENT_CONSENSUS_CHANGE,
+    TxValidationResult::TX_INPUTS_NOT_STANDARD,
+    TxValidationResult::TX_NOT_STANDARD,
+    TxValidationResult::TX_MISSING_INPUTS,
+    TxValidationResult::TX_PREMATURE_SPEND,
+    TxValidationResult::TX_WITNESS_MUTATED,
+    TxValidationResult::TX_WITNESS_STRIPPED,
+    TxValidationResult::TX_CONFLICT,
+    TxValidationResult::TX_MEMPOOL_POLICY,
+    // Skip TX_NO_MEMPOOL
+    TxValidationResult::TX_RECONSIDERABLE,
+    TxValidationResult::TX_UNKNOWN,
+};
+
+// Precomputed transactions. Some may conflict with each other.
+std::vector<CTransactionRef> TRANSACTIONS;
+
+// Limit the total number of peers because we don't expect coverage to change much with lots more peers.
+constexpr int NUM_PEERS = 16;
+
+// Precomputed random durations (positive and negative, each ~exponentially distributed).
+std::chrono::microseconds TIME_SKIPS[128];
+
+static CTransactionRef MakeTransactionSpending(const std::vector<COutPoint>& outpoints, size_t num_outputs, bool add_witness)
+{
+    CMutableTransaction tx;
+    // If no outpoints are given, create a random one.
+    for (const auto& outpoint : outpoints) {
+        tx.vin.emplace_back(outpoint);
+    }
+    if (add_witness) {
+        tx.vin[0].scriptWitness.stack.push_back({1});
+    }
+    for (size_t o = 0; o < num_outputs; ++o) tx.vout.emplace_back(CENT, P2WSH_OP_TRUE);
+    return MakeTransactionRef(tx);
+}
+static std::vector<COutPoint> PickCoins(FuzzedDataProvider& fuzzed_data_provider)
+{
+    std::vector<COutPoint> ret;
+    ret.push_back(fuzzed_data_provider.PickValueInArray(COINS));
+    LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10) {
+        ret.push_back(fuzzed_data_provider.PickValueInArray(COINS));
+    }
+    return ret;
+}
+
+void initialize()
+{
+    static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>();
+    g_setup = testing_setup.get();
+    for (uint32_t i = 0; i < uint32_t{NUM_COINS}; ++i) {
+        COINS[i] = COutPoint{Txid::FromUint256((HashWriter() << i).GetHash()), i};
+    }
+    size_t outpoints_index = 0;
+    // 2 transactions same txid different witness
+    {
+        auto tx1{MakeTransactionSpending({COINS[outpoints_index]}, /*num_outputs=*/5, /*add_witness=*/false)};
+        auto tx2{MakeTransactionSpending({COINS[outpoints_index]}, /*num_outputs=*/5, /*add_witness=*/true)};
+        Assert(tx1->GetHash() == tx2->GetHash());
+        TRANSACTIONS.emplace_back(tx1);
+        TRANSACTIONS.emplace_back(tx2);
+        outpoints_index += 1;
+    }
+    // 2 parents 1 child
+    {
+        auto tx_parent_1{MakeTransactionSpending({COINS[outpoints_index++]}, /*num_outputs=*/1, /*add_witness=*/true)};
+        TRANSACTIONS.emplace_back(tx_parent_1);
+        auto tx_parent_2{MakeTransactionSpending({COINS[outpoints_index++]}, /*num_outputs=*/1, /*add_witness=*/false)};
+        TRANSACTIONS.emplace_back(tx_parent_2);
+        TRANSACTIONS.emplace_back(MakeTransactionSpending({COutPoint{tx_parent_1->GetHash(), 0}, COutPoint{tx_parent_2->GetHash(), 0}},
+                                                            /*num_outputs=*/1, /*add_witness=*/true));
+    }
+    // 1 parent 2 children
+    {
+        auto tx_parent{MakeTransactionSpending({COINS[outpoints_index++]}, /*num_outputs=*/2, /*add_witness=*/true)};
+        TRANSACTIONS.emplace_back(tx_parent);
+        TRANSACTIONS.emplace_back(MakeTransactionSpending({COutPoint{tx_parent->GetHash(), 0}},
+                                                            /*num_outputs=*/1, /*add_witness=*/true));
+        TRANSACTIONS.emplace_back(MakeTransactionSpending({COutPoint{tx_parent->GetHash(), 1}},
+                                                            /*num_outputs=*/1, /*add_witness=*/true));
+    }
+    // chain of 5 segwit
+    {
+        COutPoint& last_outpoint = COINS[outpoints_index++];
+        for (auto i{0}; i < 5; ++i) {
+            auto tx{MakeTransactionSpending({last_outpoint}, /*num_outputs=*/1, /*add_witness=*/true)};
+            TRANSACTIONS.emplace_back(tx);
+            last_outpoint = COutPoint{tx->GetHash(), 0};
+        }
+    }
+    // chain of 5 non-segwit
+    {
+        COutPoint& last_outpoint = COINS[outpoints_index++];
+        for (auto i{0}; i < 5; ++i) {
+            auto tx{MakeTransactionSpending({last_outpoint}, /*num_outputs=*/1, /*add_witness=*/false)};
+            TRANSACTIONS.emplace_back(tx);
+            last_outpoint = COutPoint{tx->GetHash(), 0};
+        }
+    }
+    // Also create a loose tx for each outpoint. Some of these transactions conflict with the above
+    // or have the same txid.
+    for (const auto& outpoint : COINS) {
+        TRANSACTIONS.emplace_back(MakeTransactionSpending({outpoint}, /*num_outputs=*/1, /*add_witness=*/true));
+    }
+
+    // Create random-looking time jumps
+    int i = 0;
+    // TIME_SKIPS[N] for N=0..15 is just N microseconds.
+    for (; i < 16; ++i) {
+        TIME_SKIPS[i] = std::chrono::microseconds{i};
+    }
+    // TIME_SKIPS[N] for N=16..127 has randomly-looking but roughly exponentially increasing values up to
+    // 198.416453 seconds.
+    for (; i < 128; ++i) {
+        int diff_bits = ((i - 10) * 2) / 9;
+        uint64_t diff = 1 + (CSipHasher(0, 0).Write(i).Finalize() >> (64 - diff_bits));
+        TIME_SKIPS[i] = TIME_SKIPS[i - 1] + std::chrono::microseconds{diff};
+    }
+}
+
+void CheckPackageToValidate(const node::PackageToValidate& package_to_validate, NodeId peer)
+{
+    Assert(package_to_validate.m_senders.size() == 2);
+    Assert(package_to_validate.m_senders.front() == peer);
+    Assert(package_to_validate.m_senders.back() < NUM_PEERS);
+
+    // Package is a 1p1c
+    const auto& package = package_to_validate.m_txns;
+    Assert(IsChildWithParents(package));
+    Assert(package.size() == 2);
+}
+
+FUZZ_TARGET(txdownloadman, .init = initialize)
+{
+    FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
+
+    // Initialize txdownloadman
+    bilingual_str error;
+    CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node), error};
+    const auto max_orphan_count = fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 300);
+    FastRandomContext det_rand{true};
+    node::TxDownloadManager txdownloadman{node::TxDownloadOptions{pool, det_rand, max_orphan_count, true}};
+
+    std::chrono::microseconds time{244466666};
+
+    LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000)
+    {
+        NodeId rand_peer = fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(0, NUM_PEERS - 1);
+
+        // Transaction can be one of the premade ones or a randomly generated one
+        auto rand_tx = fuzzed_data_provider.ConsumeBool() ?
+            MakeTransactionSpending(PickCoins(fuzzed_data_provider),
+                                    /*num_outputs=*/fuzzed_data_provider.ConsumeIntegralInRange(1, 500),
+                                    /*add_witness=*/fuzzed_data_provider.ConsumeBool()) :
+            TRANSACTIONS.at(fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, TRANSACTIONS.size() - 1));
+
+        CallOneOf(
+            fuzzed_data_provider,
+            [&] {
+                node::TxDownloadConnectionInfo info{
+                    .m_preferred = fuzzed_data_provider.ConsumeBool(),
+                    .m_relay_permissions = fuzzed_data_provider.ConsumeBool(),
+                    .m_wtxid_relay = fuzzed_data_provider.ConsumeBool()
+                };
+                txdownloadman.ConnectedPeer(rand_peer, info);
+            },
+            [&] {
+                txdownloadman.DisconnectedPeer(rand_peer);
+                txdownloadman.CheckIsEmpty(rand_peer);
+            },
+            [&] {
+                txdownloadman.ActiveTipChange();
+            },
+            [&] {
+                CBlock block;
+                block.vtx.push_back(rand_tx);
+                txdownloadman.BlockConnected(std::make_shared<CBlock>(block));
+            },
+            [&] {
+                txdownloadman.BlockDisconnected();
+            },
+            [&] {
+                txdownloadman.MempoolAcceptedTx(rand_tx);
+            },
+            [&] {
+                TxValidationState state;
+                state.Invalid(fuzzed_data_provider.PickValueInArray(TESTED_TX_RESULTS), "");
+                bool first_time_failure{fuzzed_data_provider.ConsumeBool()};
+
+                node::RejectedTxTodo todo = txdownloadman.MempoolRejectedTx(rand_tx, state, rand_peer, first_time_failure);
+                Assert(first_time_failure || !todo.m_should_add_extra_compact_tx);
+            },
+            [&] {
+                GenTxid gtxid = fuzzed_data_provider.ConsumeBool() ?
+                                GenTxid::Txid(rand_tx->GetHash()) :
+                                GenTxid::Wtxid(rand_tx->GetWitnessHash());
+                txdownloadman.AddTxAnnouncement(rand_peer, gtxid, time, /*p2p_inv=*/fuzzed_data_provider.ConsumeBool());
+            },
+            [&] {
+                txdownloadman.GetRequestsToSend(rand_peer, time);
+            },
+            [&] {
+                txdownloadman.ReceivedTx(rand_peer, rand_tx);
+                const auto& [should_validate, maybe_package] = txdownloadman.ReceivedTx(rand_peer, rand_tx);
+                // The only possible results should be:
+                // - Don't validate the tx, no package.
+                // - Don't validate the tx, package.
+                // - Validate the tx, no package.
+                // The only combination that doesn't make sense is validate both tx and package.
+                Assert(!(should_validate && maybe_package.has_value()));
+                if (maybe_package.has_value()) CheckPackageToValidate(*maybe_package, rand_peer);
+            },
+            [&] {
+                txdownloadman.ReceivedNotFound(rand_peer, {rand_tx->GetWitnessHash()});
+            },
+            [&] {
+                const bool expect_work{txdownloadman.HaveMoreWork(rand_peer)};
+                const auto ptx = txdownloadman.GetTxToReconsider(rand_peer);
+                // expect_work=true doesn't necessarily mean the next item from the workset isn't a
+                // nullptr, as the transaction could have been removed from orphanage without being
+                // removed from the peer's workset.
+                if (ptx) {
+                    // However, if there was a non-null tx in the workset, HaveMoreWork should have
+                    // returned true.
+                    Assert(expect_work);
+                }
+            }
+        );
+        // Jump forwards or backwards
+        auto time_skip = fuzzed_data_provider.PickValueInArray(TIME_SKIPS);
+        if (fuzzed_data_provider.ConsumeBool()) time_skip *= -1;
+        time += time_skip;
+    }
+    // Disconnect everybody, check that all data structures are empty.
+    for (NodeId nodeid = 0; nodeid < NUM_PEERS; ++nodeid) {
+        txdownloadman.DisconnectedPeer(nodeid);
+        txdownloadman.CheckIsEmpty(nodeid);
+    }
+    txdownloadman.CheckIsEmpty();
+}
+
+// Give node 0 relay permissions, and nobody else. This helps us remember who is a RelayPermissions
+// peer without tracking anything (this is only for the txdownload_impl target).
+static bool HasRelayPermissions(NodeId peer) { return peer == 0; }
+
+static void CheckInvariants(const node::TxDownloadManagerImpl& txdownload_impl, size_t max_orphan_count)
+{
+    const TxOrphanage& orphanage = txdownload_impl.m_orphanage;
+
+    // Orphanage usage should never exceed what is allowed
+    Assert(orphanage.Size() <= max_orphan_count);
+
+    // We should never have more than the maximum in-flight requests out for a peer.
+    for (NodeId peer = 0; peer < NUM_PEERS; ++peer) {
+        if (!HasRelayPermissions(peer)) {
+            Assert(txdownload_impl.m_txrequest.CountInFlight(peer) <= node::MAX_PEER_TX_REQUEST_IN_FLIGHT);
+        }
+    }
+    txdownload_impl.m_txrequest.SanityCheck();
+}
+
+FUZZ_TARGET(txdownloadman_impl, .init = initialize)
+{
+    FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
+
+    // Initialize a TxDownloadManagerImpl
+    bilingual_str error;
+    CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node), error};
+    const auto max_orphan_count = fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 300);
+    FastRandomContext det_rand{true};
+    node::TxDownloadManagerImpl txdownload_impl{node::TxDownloadOptions{pool, det_rand, max_orphan_count, true}};
+
+    std::chrono::microseconds time{244466666};
+
+    LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000)
+    {
+        NodeId rand_peer = fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(0, NUM_PEERS - 1);
+
+        // Transaction can be one of the premade ones or a randomly generated one
+        auto rand_tx = fuzzed_data_provider.ConsumeBool() ?
+            MakeTransactionSpending(PickCoins(fuzzed_data_provider),
+                                    /*num_outputs=*/fuzzed_data_provider.ConsumeIntegralInRange(1, 500),
+                                    /*add_witness=*/fuzzed_data_provider.ConsumeBool()) :
+            TRANSACTIONS.at(fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, TRANSACTIONS.size() - 1));
+
+        CallOneOf(
+            fuzzed_data_provider,
+            [&] {
+                node::TxDownloadConnectionInfo info{
+                    .m_preferred = fuzzed_data_provider.ConsumeBool(),
+                    .m_relay_permissions = HasRelayPermissions(rand_peer),
+                    .m_wtxid_relay = fuzzed_data_provider.ConsumeBool()
+                };
+                txdownload_impl.ConnectedPeer(rand_peer, info);
+            },
+            [&] {
+                txdownload_impl.DisconnectedPeer(rand_peer);
+                txdownload_impl.CheckIsEmpty(rand_peer);
+            },
+            [&] {
+                txdownload_impl.ActiveTipChange();
+                // After a block update, nothing should be in the rejection caches
+                for (const auto& tx : TRANSACTIONS) {
+                    Assert(!txdownload_impl.RecentRejectsFilter().contains(tx->GetWitnessHash().ToUint256()));
+                    Assert(!txdownload_impl.RecentRejectsFilter().contains(tx->GetHash().ToUint256()));
+                    Assert(!txdownload_impl.RecentRejectsReconsiderableFilter().contains(tx->GetWitnessHash().ToUint256()));
+                    Assert(!txdownload_impl.RecentRejectsReconsiderableFilter().contains(tx->GetHash().ToUint256()));
+                }
+            },
+            [&] {
+                CBlock block;
+                block.vtx.push_back(rand_tx);
+                txdownload_impl.BlockConnected(std::make_shared<CBlock>(block));
+                // Block transactions must be removed from orphanage
+                Assert(!txdownload_impl.m_orphanage.HaveTx(rand_tx->GetWitnessHash()));
+            },
+            [&] {
+                txdownload_impl.BlockDisconnected();
+                Assert(!txdownload_impl.RecentConfirmedTransactionsFilter().contains(rand_tx->GetWitnessHash().ToUint256()));
+                Assert(!txdownload_impl.RecentConfirmedTransactionsFilter().contains(rand_tx->GetHash().ToUint256()));
+            },
+            [&] {
+                txdownload_impl.MempoolAcceptedTx(rand_tx);
+            },
+            [&] {
+                TxValidationState state;
+                state.Invalid(fuzzed_data_provider.PickValueInArray(TESTED_TX_RESULTS), "");
+                bool first_time_failure{fuzzed_data_provider.ConsumeBool()};
+
+                bool reject_contains_wtxid{txdownload_impl.RecentRejectsFilter().contains(rand_tx->GetWitnessHash().ToUint256())};
+
+                node::RejectedTxTodo todo = txdownload_impl.MempoolRejectedTx(rand_tx, state, rand_peer, first_time_failure);
+                Assert(first_time_failure || !todo.m_should_add_extra_compact_tx);
+                if (!reject_contains_wtxid) Assert(todo.m_unique_parents.size() <= rand_tx->vin.size());
+            },
+            [&] {
+                GenTxid gtxid = fuzzed_data_provider.ConsumeBool() ?
+                                GenTxid::Txid(rand_tx->GetHash()) :
+                                GenTxid::Wtxid(rand_tx->GetWitnessHash());
+                txdownload_impl.AddTxAnnouncement(rand_peer, gtxid, time, /*p2p_inv=*/fuzzed_data_provider.ConsumeBool());
+            },
+            [&] {
+                const auto getdata_requests = txdownload_impl.GetRequestsToSend(rand_peer, time);
+                // TxDownloadManager should not be telling us to request things we already have.
+                // Exclude m_lazy_recent_rejects_reconsiderable because it may request low-feerate parent of orphan.
+                for (const auto& gtxid : getdata_requests) {
+                    Assert(!txdownload_impl.AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false));
+                }
+            },
+            [&] {
+                const auto& [should_validate, maybe_package] = txdownload_impl.ReceivedTx(rand_peer, rand_tx);
+                // The only possible results should be:
+                // - Don't validate the tx, no package.
+                // - Don't validate the tx, package.
+                // - Validate the tx, no package.
+                // The only combination that doesn't make sense is validate both tx and package.
+                Assert(!(should_validate && maybe_package.has_value()));
+                if (should_validate) {
+                    Assert(!txdownload_impl.AlreadyHaveTx(GenTxid::Wtxid(rand_tx->GetWitnessHash()), /*include_reconsiderable=*/true));
+                }
+                if (maybe_package.has_value()) {
+                    CheckPackageToValidate(*maybe_package, rand_peer);
+
+                    const auto& package = maybe_package->m_txns;
+                    // Parent is in m_lazy_recent_rejects_reconsiderable and child is in m_orphanage
+                    Assert(txdownload_impl.RecentRejectsReconsiderableFilter().contains(rand_tx->GetWitnessHash().ToUint256()));
+                    Assert(txdownload_impl.m_orphanage.HaveTx(maybe_package->m_txns.back()->GetWitnessHash()));
+                    // Package has not been rejected
+                    Assert(!txdownload_impl.RecentRejectsReconsiderableFilter().contains(GetPackageHash(package)));
+                    // Neither is in m_lazy_recent_rejects
+                    Assert(!txdownload_impl.RecentRejectsFilter().contains(package.front()->GetWitnessHash().ToUint256()));
+                    Assert(!txdownload_impl.RecentRejectsFilter().contains(package.back()->GetWitnessHash().ToUint256()));
+                }
+            },
+            [&] {
+                txdownload_impl.ReceivedNotFound(rand_peer, {rand_tx->GetWitnessHash()});
+            },
+            [&] {
+                const bool expect_work{txdownload_impl.HaveMoreWork(rand_peer)};
+                const auto ptx{txdownload_impl.GetTxToReconsider(rand_peer)};
+                // expect_work=true doesn't necessarily mean the next item from the workset isn't a
+                // nullptr, as the transaction could have been removed from orphanage without being
+                // removed from the peer's workset.
+                if (ptx) {
+                    // However, if there was a non-null tx in the workset, HaveMoreWork should have
+                    // returned true.
+                    Assert(expect_work);
+                    Assert(txdownload_impl.AlreadyHaveTx(GenTxid::Wtxid(ptx->GetWitnessHash()), /*include_reconsiderable=*/false));
+                    // Presumably we have validated this tx. Use "missing inputs" to keep it in the
+                    // orphanage longer. Later iterations might call MempoolAcceptedTx or
+                    // MempoolRejectedTx with a different error.
+                    TxValidationState state_missing_inputs;
+                    state_missing_inputs.Invalid(TxValidationResult::TX_MISSING_INPUTS, "");
+                    txdownload_impl.MempoolRejectedTx(ptx, state_missing_inputs, rand_peer, fuzzed_data_provider.ConsumeBool());
+                }
+            }
+        );
+
+        // Jump ahead in time
+        time += fuzzed_data_provider.PickValueInArray(TIME_SKIPS);
+        CheckInvariants(txdownload_impl, max_orphan_count);
+    }
+    // Disconnect everybody, check that all data structures are empty.
+    for (NodeId nodeid = 0; nodeid < NUM_PEERS; ++nodeid) {
+        txdownload_impl.DisconnectedPeer(nodeid);
+        txdownload_impl.CheckIsEmpty(nodeid);
+    }
+    txdownload_impl.CheckIsEmpty();
+}
+
+} // namespace