From 6f4da19cc3b1b7cd23cb4be95a6bb9acb79eb3bf Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 13 Mar 2024 11:25:24 +0000
Subject: [PATCH 1/9] guard against
 MempoolAcceptResult::m_replaced_transactions

It should never be a nullopt when the transaction result is valid -
Assume() this is the case. However, as a belt-and-suspenders just in
case it is nullopt, use an empty list.
---
 src/net_processing.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 39ffff97d2..07ac1cfec7 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -4354,7 +4354,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         const TxValidationState& state = result.m_state;
 
         if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
-            ProcessValidTx(pfrom.GetId(), ptx, result.m_replaced_transactions.value());
+            Assume(result.m_replaced_transactions.has_value());
+            std::list<CTransactionRef> empty_replacement_list;
+            ProcessValidTx(pfrom.GetId(), ptx, result.m_replaced_transactions.value_or(empty_replacement_list));
             pfrom.m_last_tx_time = GetTime<std::chrono::seconds>();
         }
         else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS)

From c3c1e15831c463df7968b028a77e787da7e6256d Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 13 Mar 2024 11:45:33 +0000
Subject: [PATCH 2/9] [doc] restore comment about why we check if ptx
 HasWitness before caching rejected txid

---
 src/net_processing.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 07ac1cfec7..577ed5cd3f 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3107,6 +3107,8 @@ void PeerManagerImpl::ProcessInvalidTx(NodeId nodeid, const CTransactionRef& ptx
         // processing of this transaction in the event that child
         // transactions are later received (resulting in
         // parent-fetching by txid via the orphan-handling logic).
+        // We only add the txid if it differs from the wtxid, to avoid wasting entries in the
+        // rolling bloom filter.
         if (state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && ptx->HasWitness()) {
             m_recent_rejects.insert(ptx->GetHash().ToUint256());
             m_txrequest.ForgetTxHash(ptx->GetHash());

From 092c978a42e8f4a02291b994713505ba8aac8b28 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Tue, 16 Apr 2024 11:58:25 +0100
Subject: [PATCH 3/9] [txpackages] add canonical way to get hash of package

---
 src/policy/packages.cpp      | 18 +++++++
 src/policy/packages.h        |  5 ++
 src/test/txpackage_tests.cpp | 98 +++++++++++++++++++++++++++++++++++-
 3 files changed, 120 insertions(+), 1 deletion(-)

diff --git a/src/policy/packages.cpp b/src/policy/packages.cpp
index 3a63a9fe46..99d2a6d514 100644
--- a/src/policy/packages.cpp
+++ b/src/policy/packages.cpp
@@ -147,3 +147,21 @@ bool IsChildWithParentsTree(const Package& package)
         return true;
     });
 }
+
+uint256 GetPackageHash(const std::vector<CTransactionRef>& transactions)
+{
+    // Create a vector of the wtxids.
+    std::vector<Wtxid> wtxids_copy;
+    std::transform(transactions.cbegin(), transactions.cend(), std::back_inserter(wtxids_copy),
+        [](const auto& tx){ return tx->GetWitnessHash(); });
+
+    // Sort in ascending order
+    std::sort(wtxids_copy.begin(), wtxids_copy.end(), [](const auto& lhs, const auto& rhs) { return lhs.GetHex() < rhs.GetHex(); });
+
+    // Get sha256 hash of the wtxids concatenated in this order
+    HashWriter hashwriter;
+    for (const auto& wtxid : wtxids_copy) {
+        hashwriter << wtxid;
+    }
+    return hashwriter.GetSHA256();
+}
diff --git a/src/policy/packages.h b/src/policy/packages.h
index 537d8476e2..3050320122 100644
--- a/src/policy/packages.h
+++ b/src/policy/packages.h
@@ -88,4 +88,9 @@ bool IsChildWithParents(const Package& package);
  * other (the package is a "tree").
  */
 bool IsChildWithParentsTree(const Package& package);
+
+/** Get the hash of these transactions' wtxids, concatenated in lexicographical order (treating the
+ * wtxids as little endian encoded uint256, smallest to largest). */
+uint256 GetPackageHash(const std::vector<CTransactionRef>& transactions);
+
 #endif // BITCOIN_POLICY_PACKAGES_H
diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp
index b948ea8acb..8112f5f685 100644
--- a/src/test/txpackage_tests.cpp
+++ b/src/test/txpackage_tests.cpp
@@ -8,9 +8,12 @@
 #include <policy/policy.h>
 #include <primitives/transaction.h>
 #include <script/script.h>
+#include <serialize.h>
+#include <streams.h>
 #include <test/util/random.h>
 #include <test/util/script.h>
 #include <test/util/setup_common.h>
+#include <util/strencodings.h>
 #include <test/util/txmempool.h>
 #include <validation.h>
 
@@ -40,6 +43,93 @@ inline CTransactionRef create_placeholder_tx(size_t num_inputs, size_t num_outpu
     return MakeTransactionRef(mtx);
 }
 
+// Create a Wtxid from a hex string
+inline Wtxid WtxidFromString(std::string_view str)
+{
+    return Wtxid::FromUint256(uint256S(str.data()));
+}
+
+BOOST_FIXTURE_TEST_CASE(package_hash_tests, TestChain100Setup)
+{
+    // Random real segwit transaction
+    DataStream stream_1{
+        ParseHex("02000000000101964b8aa63509579ca6086e6012eeaa4c2f4dd1e283da29b67c8eea38b3c6fd220000000000fdffffff0294c618000000000017a9145afbbb42f4e83312666d0697f9e66259912ecde38768fa2c0000000000160014897388a0889390fd0e153a22bb2cf9d8f019faf50247304402200547406380719f84d68cf4e96cc3e4a1688309ef475b150be2b471c70ea562aa02206d255f5acc40fd95981874d77201d2eb07883657ce1c796513f32b6079545cdf0121023ae77335cefcb5ab4c1dc1fb0d2acfece184e593727d7d5906c78e564c7c11d125cf0c00"),
+    };
+    CTransaction tx_1(deserialize, TX_WITH_WITNESS, stream_1);
+    CTransactionRef ptx_1{MakeTransactionRef(tx_1)};
+
+    // Random real nonsegwit transaction
+    DataStream stream_2{
+        ParseHex("01000000010b26e9b7735eb6aabdf358bab62f9816a21ba9ebdb719d5299e88607d722c190000000008b4830450220070aca44506c5cef3a16ed519d7c3c39f8aab192c4e1c90d065f37b8a4af6141022100a8e160b856c2d43d27d8fba71e5aef6405b8643ac4cb7cb3c462aced7f14711a0141046d11fee51b0e60666d5049a9101a72741df480b96ee26488a4d3466b95c9a40ac5eeef87e10a5cd336c19a84565f80fa6c547957b7700ff4dfbdefe76036c339ffffffff021bff3d11000000001976a91404943fdd508053c75000106d3bc6e2754dbcff1988ac2f15de00000000001976a914a266436d2965547608b9e15d9032a7b9d64fa43188ac00000000"),
+    };
+    CTransaction tx_2(deserialize, TX_WITH_WITNESS, stream_2);
+    CTransactionRef ptx_2{MakeTransactionRef(tx_2)};
+
+    // Random real segwit transaction
+    DataStream stream_3{
+        ParseHex("0200000000010177862801f77c2c068a70372b4c435ef8dd621291c36a64eb4dd491f02218f5324600000000fdffffff014a0100000000000022512035ea312034cfac01e956a269f3bf147f569c2fbb00180677421262da042290d803402be713325ff285e66b0380f53f2fae0d0fb4e16f378a440fed51ce835061437566729d4883bc917632f3cff474d6384bc8b989961a1d730d4a87ed38ad28bd337b20f1d658c6c138b1c312e072b4446f50f01ae0da03a42e6274f8788aae53416a7fac0063036f7264010118746578742f706c61696e3b636861727365743d7574662d3800357b2270223a226272632d3230222c226f70223a226d696e74222c227469636b223a224342414c222c22616d74223a2236393639227d6821c1f1d658c6c138b1c312e072b4446f50f01ae0da03a42e6274f8788aae53416a7f00000000"),
+    };
+    CTransaction tx_3(deserialize, TX_WITH_WITNESS, stream_3);
+    CTransactionRef ptx_3{MakeTransactionRef(tx_3)};
+
+    // It's easy to see that wtxids are sorted in lexicographical order:
+    Wtxid wtxid_1{WtxidFromString("0x85cd1a31eb38f74ed5742ec9cb546712ab5aaf747de28a9168b53e846cbda17f")};
+    Wtxid wtxid_2{WtxidFromString("0xb4749f017444b051c44dfd2720e88f314ff94f3dd6d56d40ef65854fcd7fff6b")};
+    Wtxid wtxid_3{WtxidFromString("0xe065bac15f62bb4e761d761db928ddee65a47296b2b776785abb912cdec474e3")};
+    BOOST_CHECK_EQUAL(tx_1.GetWitnessHash(), wtxid_1);
+    BOOST_CHECK_EQUAL(tx_2.GetWitnessHash(), wtxid_2);
+    BOOST_CHECK_EQUAL(tx_3.GetWitnessHash(), wtxid_3);
+
+    BOOST_CHECK(wtxid_1.GetHex() < wtxid_2.GetHex());
+    BOOST_CHECK(wtxid_2.GetHex() < wtxid_3.GetHex());
+
+    // The txids are not (we want to test that sorting and hashing use wtxid, not txid):
+    Txid txid_1{TxidFromString("0xbd0f71c1d5e50589063e134fad22053cdae5ab2320db5bf5e540198b0b5a4e69")};
+    Txid txid_2{TxidFromString("0xb4749f017444b051c44dfd2720e88f314ff94f3dd6d56d40ef65854fcd7fff6b")};
+    Txid txid_3{TxidFromString("0xee707be5201160e32c4fc715bec227d1aeea5940fb4295605e7373edce3b1a93")};
+    BOOST_CHECK_EQUAL(tx_1.GetHash(), txid_1);
+    BOOST_CHECK_EQUAL(tx_2.GetHash(), txid_2);
+    BOOST_CHECK_EQUAL(tx_3.GetHash(), txid_3);
+
+    BOOST_CHECK(txid_2.GetHex() < txid_1.GetHex());
+
+    BOOST_CHECK(txid_1.ToUint256() != wtxid_1.ToUint256());
+    BOOST_CHECK(txid_2.ToUint256() == wtxid_2.ToUint256());
+    BOOST_CHECK(txid_3.ToUint256() != wtxid_3.ToUint256());
+
+    // We are testing that both functions compare using GetHex() and not uint256.
+    // (in this pair of wtxids, hex string order != uint256 order)
+    BOOST_CHECK(wtxid_2 < wtxid_1);
+    // (in this pair of wtxids, hex string order == uint256 order)
+    BOOST_CHECK(wtxid_2 < wtxid_3);
+
+    // All permutations of the package containing ptx_1, ptx_2, ptx_3 have the same package hash
+    std::vector<CTransactionRef> package_123{ptx_1, ptx_2, ptx_3};
+    std::vector<CTransactionRef> package_132{ptx_1, ptx_3, ptx_2};
+    std::vector<CTransactionRef> package_231{ptx_2, ptx_3, ptx_1};
+    std::vector<CTransactionRef> package_213{ptx_2, ptx_1, ptx_3};
+    std::vector<CTransactionRef> package_312{ptx_3, ptx_1, ptx_2};
+    std::vector<CTransactionRef> package_321{ptx_3, ptx_2, ptx_1};
+
+    uint256 calculated_hash_123 = (HashWriter() << wtxid_1 << wtxid_2 << wtxid_3).GetSHA256();
+
+    uint256 hash_if_by_txid = (HashWriter() << wtxid_2 << wtxid_1 << wtxid_3).GetSHA256();
+    BOOST_CHECK(hash_if_by_txid != calculated_hash_123);
+
+    uint256 hash_if_use_txid = (HashWriter() << txid_2 << txid_1 << txid_3).GetSHA256();
+    BOOST_CHECK(hash_if_use_txid != calculated_hash_123);
+
+    uint256 hash_if_use_int_order = (HashWriter() << wtxid_2 << wtxid_1 << wtxid_3).GetSHA256();
+    BOOST_CHECK(hash_if_use_int_order != calculated_hash_123);
+
+    BOOST_CHECK_EQUAL(calculated_hash_123, GetPackageHash(package_123));
+    BOOST_CHECK_EQUAL(calculated_hash_123, GetPackageHash(package_132));
+    BOOST_CHECK_EQUAL(calculated_hash_123, GetPackageHash(package_231));
+    BOOST_CHECK_EQUAL(calculated_hash_123, GetPackageHash(package_213));
+    BOOST_CHECK_EQUAL(calculated_hash_123, GetPackageHash(package_312));
+    BOOST_CHECK_EQUAL(calculated_hash_123, GetPackageHash(package_321));
+}
+
 BOOST_FIXTURE_TEST_CASE(package_sanitization_tests, TestChain100Setup)
 {
     // Packages can't have more than 25 transactions.
@@ -190,6 +280,9 @@ BOOST_FIXTURE_TEST_CASE(noncontextual_package_tests, TestChain100Setup)
         BOOST_CHECK_EQUAL(state.GetRejectReason(), "package-not-sorted");
         BOOST_CHECK(IsChildWithParents({tx_parent, tx_child}));
         BOOST_CHECK(IsChildWithParentsTree({tx_parent, tx_child}));
+        BOOST_CHECK(GetPackageHash({tx_parent}) != GetPackageHash({tx_child}));
+        BOOST_CHECK(GetPackageHash({tx_child, tx_child}) != GetPackageHash({tx_child}));
+        BOOST_CHECK(GetPackageHash({tx_child, tx_parent}) != GetPackageHash({tx_child, tx_child}));
     }
 
     // 24 Parents and 1 Child
@@ -450,6 +543,8 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
     BOOST_CHECK_EQUAL(ptx_child1->GetHash(), ptx_child2->GetHash());
     // child1 and child2 have different wtxids
     BOOST_CHECK(ptx_child1->GetWitnessHash() != ptx_child2->GetWitnessHash());
+    // Check that they have different package hashes
+    BOOST_CHECK(GetPackageHash({ptx_parent, ptx_child1}) != GetPackageHash({ptx_parent, ptx_child2}));
 
     // Try submitting Package1{parent, child1} and Package2{parent, child2} where the children are
     // same-txid-different-witness.
@@ -503,7 +598,8 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
                                                         /*output_destination=*/grandchild_locking_script,
                                                         /*output_amount=*/CAmount(47 * COIN), /*submit=*/false);
     CTransactionRef ptx_grandchild = MakeTransactionRef(mtx_grandchild);
-
+    // Check that they have different package hashes
+    BOOST_CHECK(GetPackageHash({ptx_child1, ptx_grandchild}) != GetPackageHash({ptx_child2, ptx_grandchild}));
     // We already submitted child1 above.
     {
         Package package_child2_grandchild{ptx_child2, ptx_grandchild};

From 2f51cd680fb4323f1c792dae37d4c4e0e0e35804 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Thu, 28 Mar 2024 17:14:38 +0000
Subject: [PATCH 4/9] [txorphanage] add method to get all orphans spending a tx

---
 src/txorphanage.cpp | 74 +++++++++++++++++++++++++++++++++++++++++++++
 src/txorphanage.h   |  8 +++++
 2 files changed, 82 insertions(+)

diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index e907fffd4f..82206e56c3 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -241,3 +241,77 @@ void TxOrphanage::EraseForBlock(const CBlock& block)
         LogPrint(BCLog::TXPACKAGES, "Erased %d orphan tx included or conflicted by block\n", nErased);
     }
 }
+
+std::vector<CTransactionRef> TxOrphanage::GetChildrenFromSamePeer(const CTransactionRef& parent, NodeId nodeid) const
+{
+    LOCK(m_mutex);
+
+    // First construct a vector of iterators to ensure we do not return duplicates of the same tx
+    // and so we can sort by nTimeExpire.
+    std::vector<OrphanMap::iterator> iters;
+
+    // For each output, get all entries spending this prevout, filtering for ones from the specified peer.
+    for (unsigned int i = 0; i < parent->vout.size(); i++) {
+        const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(parent->GetHash(), i));
+        if (it_by_prev != m_outpoint_to_orphan_it.end()) {
+            for (const auto& elem : it_by_prev->second) {
+                if (elem->second.fromPeer == nodeid) {
+                    iters.emplace_back(elem);
+                }
+            }
+        }
+    }
+
+    // Sort by address so that duplicates can be deleted. At the same time, sort so that more recent
+    // orphans (which expire later) come first.  Break ties based on address, as nTimeExpire is
+    // quantified in seconds and it is possible for orphans to have the same expiry.
+    std::sort(iters.begin(), iters.end(), [](const auto& lhs, const auto& rhs) {
+        if (lhs->second.nTimeExpire == rhs->second.nTimeExpire) {
+            return &(*lhs) < &(*rhs);
+        } else {
+            return lhs->second.nTimeExpire > rhs->second.nTimeExpire;
+        }
+    });
+    // Erase duplicates
+    iters.erase(std::unique(iters.begin(), iters.end()), iters.end());
+
+    // Convert to a vector of CTransactionRef
+    std::vector<CTransactionRef> children_found;
+    children_found.reserve(iters.size());
+    for (const auto child_iter : iters) {
+        children_found.emplace_back(child_iter->second.tx);
+    }
+    return children_found;
+}
+
+std::vector<std::pair<CTransactionRef, NodeId>> TxOrphanage::GetChildrenFromDifferentPeer(const CTransactionRef& parent, NodeId nodeid) const
+{
+    LOCK(m_mutex);
+
+    // First construct vector of iterators to ensure we do not return duplicates of the same tx.
+    std::vector<OrphanMap::iterator> iters;
+
+    // For each output, get all entries spending this prevout, filtering for ones not from the specified peer.
+    for (unsigned int i = 0; i < parent->vout.size(); i++) {
+        const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(parent->GetHash(), i));
+        if (it_by_prev != m_outpoint_to_orphan_it.end()) {
+            for (const auto& elem : it_by_prev->second) {
+                if (elem->second.fromPeer != nodeid) {
+                    iters.emplace_back(elem);
+                }
+            }
+        }
+    }
+
+    // Erase duplicates
+    std::sort(iters.begin(), iters.end(), IteratorComparator());
+    iters.erase(std::unique(iters.begin(), iters.end()), iters.end());
+
+    // Convert iterators to pair<CTransactionRef, NodeId>
+    std::vector<std::pair<CTransactionRef, NodeId>> children_found;
+    children_found.reserve(iters.size());
+    for (const auto child_iter : iters) {
+        children_found.emplace_back(child_iter->second.tx, child_iter->second.fromPeer);
+    }
+    return children_found;
+}
diff --git a/src/txorphanage.h b/src/txorphanage.h
index 2fd14e6fd2..a3c8edaa2a 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -51,6 +51,14 @@ public:
     /** Does this peer have any work to do? */
     bool HaveTxToReconsider(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);;
 
+    /** Get all children that spend from this tx and were received from nodeid. Sorted from most
+     * recent to least recent. */
+    std::vector<CTransactionRef> GetChildrenFromSamePeer(const CTransactionRef& parent, NodeId nodeid) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+
+    /** Get all children that spend from this tx but were not received from nodeid. Also return
+     * which peer provided each tx. */
+    std::vector<std::pair<CTransactionRef, NodeId>> GetChildrenFromDifferentPeer(const CTransactionRef& parent, NodeId nodeid) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+
     /** Return how many entries exist in the orphange */
     size_t Size() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
     {

From d095316c1c23e9460dfbd9fdbaf292063adcd080 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Thu, 28 Mar 2024 17:14:50 +0000
Subject: [PATCH 5/9] [unit test] TxOrphanage::GetChildrenFrom*

---
 src/test/orphanage_tests.cpp | 147 ++++++++++++++++++++++++++++++++++-
 1 file changed, 145 insertions(+), 2 deletions(-)

diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
index 4231fcc909..b2643cf678 100644
--- a/src/test/orphanage_tests.cpp
+++ b/src/test/orphanage_tests.cpp
@@ -38,14 +38,56 @@ public:
     }
 };
 
-static void MakeNewKeyWithFastRandomContext(CKey& key)
+static void MakeNewKeyWithFastRandomContext(CKey& key, FastRandomContext& rand_ctx = g_insecure_rand_ctx)
 {
     std::vector<unsigned char> keydata;
-    keydata = g_insecure_rand_ctx.randbytes(32);
+    keydata = rand_ctx.randbytes(32);
     key.Set(keydata.data(), keydata.data() + keydata.size(), /*fCompressedIn=*/true);
     assert(key.IsValid());
 }
 
+// Creates a transaction with 2 outputs. Spends all outpoints. If outpoints is empty, spends a random one.
+static CTransactionRef MakeTransactionSpending(const std::vector<COutPoint>& outpoints, FastRandomContext& det_rand)
+{
+    CKey key;
+    MakeNewKeyWithFastRandomContext(key, det_rand);
+    CMutableTransaction tx;
+    // If no outpoints are given, create a random one.
+    if (outpoints.empty()) {
+        tx.vin.emplace_back(Txid::FromUint256(det_rand.rand256()), 0);
+    } else {
+        for (const auto& outpoint : outpoints) {
+            tx.vin.emplace_back(outpoint);
+        }
+    }
+    // Ensure txid != wtxid
+    tx.vin[0].scriptWitness.stack.push_back({1});
+    tx.vout.resize(2);
+    tx.vout[0].nValue = CENT;
+    tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
+    tx.vout[1].nValue = 3 * CENT;
+    tx.vout[1].scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(key.GetPubKey()));
+    return MakeTransactionRef(tx);
+}
+
+static bool EqualTxns(const std::set<CTransactionRef>& set_txns, const std::vector<CTransactionRef>& vec_txns)
+{
+    if (vec_txns.size() != set_txns.size()) return false;
+    for (const auto& tx : vec_txns) {
+        if (!set_txns.contains(tx)) return false;
+    }
+    return true;
+}
+static bool EqualTxns(const std::set<CTransactionRef>& set_txns,
+                      const std::vector<std::pair<CTransactionRef, NodeId>>& vec_txns)
+{
+    if (vec_txns.size() != set_txns.size()) return false;
+    for (const auto& [tx, nodeid] : vec_txns) {
+        if (!set_txns.contains(tx)) return false;
+    }
+    return true;
+}
+
 BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
 {
     // This test had non-deterministic coverage due to
@@ -138,4 +180,105 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
     BOOST_CHECK(orphanage.CountOrphans() == 0);
 }
 
+BOOST_AUTO_TEST_CASE(get_children)
+{
+    FastRandomContext det_rand{true};
+    std::vector<COutPoint> empty_outpoints;
+
+    auto parent1 = MakeTransactionSpending(empty_outpoints, det_rand);
+    auto parent2 = MakeTransactionSpending(empty_outpoints, det_rand);
+
+    // Make sure these parents have different txids otherwise this test won't make sense.
+    while (parent1->GetHash() == parent2->GetHash()) {
+        parent2 = MakeTransactionSpending(empty_outpoints, det_rand);
+    }
+
+    // Create children to go into orphanage.
+    auto child_p1n0 = MakeTransactionSpending({{parent1->GetHash(), 0}}, det_rand);
+    auto child_p2n1 = MakeTransactionSpending({{parent2->GetHash(), 1}}, det_rand);
+    // Spends the same tx twice. Should not cause duplicates.
+    auto child_p1n0_p1n1 = MakeTransactionSpending({{parent1->GetHash(), 0}, {parent1->GetHash(), 1}}, det_rand);
+    // Spends the same outpoint as previous tx. Should still be returned; don't assume outpoints are unique.
+    auto child_p1n0_p2n0 = MakeTransactionSpending({{parent1->GetHash(), 0}, {parent2->GetHash(), 0}}, det_rand);
+
+    const NodeId node1{1};
+    const NodeId node2{2};
+
+    // All orphans provided by node1
+    {
+        TxOrphanage orphanage;
+        BOOST_CHECK(orphanage.AddTx(child_p1n0, node1));
+        BOOST_CHECK(orphanage.AddTx(child_p2n1, node1));
+        BOOST_CHECK(orphanage.AddTx(child_p1n0_p1n1, node1));
+        BOOST_CHECK(orphanage.AddTx(child_p1n0_p2n0, node1));
+
+        std::set<CTransactionRef> expected_parent1_children{child_p1n0, child_p1n0_p2n0, child_p1n0_p1n1};
+        std::set<CTransactionRef> expected_parent2_children{child_p2n1, child_p1n0_p2n0};
+
+        BOOST_CHECK(EqualTxns(expected_parent1_children, orphanage.GetChildrenFromSamePeer(parent1, node1)));
+        BOOST_CHECK(EqualTxns(expected_parent2_children, orphanage.GetChildrenFromSamePeer(parent2, node1)));
+
+        BOOST_CHECK(EqualTxns(expected_parent1_children, orphanage.GetChildrenFromDifferentPeer(parent1, node2)));
+        BOOST_CHECK(EqualTxns(expected_parent2_children, orphanage.GetChildrenFromDifferentPeer(parent2, node2)));
+
+        // The peer must match
+        BOOST_CHECK(orphanage.GetChildrenFromSamePeer(parent1, node2).empty());
+        BOOST_CHECK(orphanage.GetChildrenFromSamePeer(parent2, node2).empty());
+
+        // There shouldn't be any children of this tx in the orphanage
+        BOOST_CHECK(orphanage.GetChildrenFromSamePeer(child_p1n0_p2n0, node1).empty());
+        BOOST_CHECK(orphanage.GetChildrenFromSamePeer(child_p1n0_p2n0, node2).empty());
+        BOOST_CHECK(orphanage.GetChildrenFromDifferentPeer(child_p1n0_p2n0, node1).empty());
+        BOOST_CHECK(orphanage.GetChildrenFromDifferentPeer(child_p1n0_p2n0, node2).empty());
+    }
+
+    // Orphans provided by node1 and node2
+    {
+        TxOrphanage orphanage;
+        BOOST_CHECK(orphanage.AddTx(child_p1n0, node1));
+        BOOST_CHECK(orphanage.AddTx(child_p2n1, node1));
+        BOOST_CHECK(orphanage.AddTx(child_p1n0_p1n1, node2));
+        BOOST_CHECK(orphanage.AddTx(child_p1n0_p2n0, node2));
+
+        // +----------------+---------------+----------------------------------+
+        // |                | sender=node1  |           sender=node2           |
+        // +----------------+---------------+----------------------------------+
+        // | spends parent1 | child_p1n0    | child_p1n0_p1n1, child_p1n0_p2n0 |
+        // | spends parent2 | child_p2n1    | child_p1n0_p2n0                  |
+        // +----------------+---------------+----------------------------------+
+
+        // Children of parent1 from node1:
+        {
+            std::set<CTransactionRef> expected_parent1_node1{child_p1n0};
+
+            BOOST_CHECK(EqualTxns(expected_parent1_node1, orphanage.GetChildrenFromSamePeer(parent1, node1)));
+            BOOST_CHECK(EqualTxns(expected_parent1_node1, orphanage.GetChildrenFromDifferentPeer(parent1, node2)));
+        }
+
+        // Children of parent2 from node1:
+        {
+            std::set<CTransactionRef> expected_parent2_node1{child_p2n1};
+
+            BOOST_CHECK(EqualTxns(expected_parent2_node1, orphanage.GetChildrenFromSamePeer(parent2, node1)));
+            BOOST_CHECK(EqualTxns(expected_parent2_node1, orphanage.GetChildrenFromDifferentPeer(parent2, node2)));
+        }
+
+        // Children of parent1 from node2:
+        {
+            std::set<CTransactionRef> expected_parent1_node2{child_p1n0_p1n1, child_p1n0_p2n0};
+
+            BOOST_CHECK(EqualTxns(expected_parent1_node2, orphanage.GetChildrenFromSamePeer(parent1, node2)));
+            BOOST_CHECK(EqualTxns(expected_parent1_node2, orphanage.GetChildrenFromDifferentPeer(parent1, node1)));
+        }
+
+        // Children of parent2 from node2:
+        {
+            std::set<CTransactionRef> expected_parent2_node2{child_p1n0_p2n0};
+
+            BOOST_CHECK(EqualTxns(expected_parent2_node2, orphanage.GetChildrenFromSamePeer(parent2, node2)));
+            BOOST_CHECK(EqualTxns(expected_parent2_node2, orphanage.GetChildrenFromDifferentPeer(parent2, node1)));
+        }
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()

From 410ebd6efaf20fe4715c9b825103b74db69f35ac Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Thu, 28 Mar 2024 17:15:11 +0000
Subject: [PATCH 6/9] [fuzz] break out parent functions and add
 GetChildrenFrom* coverage

It's very hard to randomly construct a transaction that would be the
parent of an existing orphanage tx. For functions like
AddChildrenToWorkSet and GetChildren that take orphan parents, use a tx
that was previously constructed.
---
 src/test/fuzz/txorphan.cpp | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp
index 5423ba8920..a44f47b00d 100644
--- a/src/test/fuzz/txorphan.cpp
+++ b/src/test/fuzz/txorphan.cpp
@@ -45,6 +45,8 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
     // if true, allow duplicate input when constructing tx
     const bool duplicate_input = fuzzed_data_provider.ConsumeBool();
 
+    CTransactionRef ptx_potential_parent = nullptr;
+
     LIMITED_WHILE(outpoints.size() < 200'000 && fuzzed_data_provider.ConsumeBool(), 10 * DEFAULT_MAX_ORPHAN_TRANSACTIONS)
     {
         // construct transaction
@@ -78,6 +80,27 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
             return new_tx;
         }();
 
+        // Trigger orphanage functions that are called using parents. ptx_potential_parent is a tx we constructed in a
+        // previous loop and potentially the parent of this tx.
+        if (ptx_potential_parent) {
+            // Set up future GetTxToReconsider call.
+            orphanage.AddChildrenToWorkSet(*ptx_potential_parent);
+
+            // Check that all txns returned from GetChildrenFrom* are indeed a direct child of this tx.
+            NodeId peer_id = fuzzed_data_provider.ConsumeIntegral<NodeId>();
+            for (const auto& child : orphanage.GetChildrenFromSamePeer(ptx_potential_parent, peer_id)) {
+                assert(std::any_of(child->vin.cbegin(), child->vin.cend(), [&](const auto& input) {
+                    return input.prevout.hash == ptx_potential_parent->GetHash();
+                }));
+            }
+            for (const auto& [child, peer] : orphanage.GetChildrenFromDifferentPeer(ptx_potential_parent, peer_id)) {
+                assert(std::any_of(child->vin.cbegin(), child->vin.cend(), [&](const auto& input) {
+                    return input.prevout.hash == ptx_potential_parent->GetHash();
+                }));
+                assert(peer != peer_id);
+            }
+        }
+
         // trigger orphanage functions
         LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10 * DEFAULT_MAX_ORPHAN_TRANSACTIONS)
         {
@@ -85,9 +108,6 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
 
             CallOneOf(
                 fuzzed_data_provider,
-                [&] {
-                    orphanage.AddChildrenToWorkSet(*tx);
-                },
                 [&] {
                     {
                         CTransactionRef ref = orphanage.GetTxToReconsider(peer_id);
@@ -136,6 +156,12 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                     orphanage.LimitOrphans(limit, limit_orphans_rng);
                     Assert(orphanage.Size() <= limit);
                 });
+
         }
+        // Set tx as potential parent to be used for future GetChildren() calls.
+        if (!ptx_potential_parent || fuzzed_data_provider.ConsumeBool()) {
+            ptx_potential_parent = tx;
+        }
+
     }
 }

From 6c51e1d7d021ed6523107a6db87a865aaa8fc4c9 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Thu, 9 Nov 2023 14:50:33 +0000
Subject: [PATCH 7/9] [p2p] add separate rejections cache for reconsiderable
 txns

---
 src/net_processing.cpp | 59 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 51 insertions(+), 8 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 577ed5cd3f..37d9e169e1 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -586,7 +586,7 @@ private:
      * @param[in]   maybe_add_extra_compact_tx    Whether this tx should be added to vExtraTxnForCompact.
      *                                            Set to false if the tx has already been rejected before,
      *                                            e.g. is an orphan, to avoid adding duplicate entries.
-     * Updates m_txrequest, m_recent_rejects, m_orphanage, and vExtraTxnForCompact. */
+     * Updates m_txrequest, m_recent_rejects, m_recent_rejects_reconsiderable, m_orphanage, and vExtraTxnForCompact. */
     void ProcessInvalidTx(NodeId nodeid, const CTransactionRef& tx, const TxValidationState& result,
                           bool maybe_add_extra_compact_tx)
         EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main);
@@ -806,7 +806,16 @@ private:
     /** Stalling timeout for blocks in IBD */
     std::atomic<std::chrono::seconds> m_block_stalling_timeout{BLOCK_STALLING_TIMEOUT_DEFAULT};
 
-    bool AlreadyHaveTx(const GenTxid& gtxid)
+    /** Check whether we already have this gtxid in:
+     *  - mempool
+     *  - orphanage
+     *  - m_recent_rejects
+     *  - m_recent_rejects_reconsiderable (if include_reconsiderable = true)
+     *  - m_recent_confirmed_transactions
+     * Also responsible for resetting m_recent_rejects and m_recent_rejects_reconsiderable if the
+     * chain tip has changed.
+     *  */
+    bool AlreadyHaveTx(const GenTxid& gtxid, bool include_reconsiderable)
         EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_recent_confirmed_transactions_mutex);
 
     /**
@@ -844,8 +853,32 @@ private:
      * Memory used: 1.3 MB
      */
     CRollingBloomFilter m_recent_rejects GUARDED_BY(::cs_main){120'000, 0.000'001};
+    /** Block hash of chain tip the last time we reset m_recent_rejects and
+     * m_recent_rejects_reconsiderable. */
     uint256 hashRecentRejectsChainTip GUARDED_BY(cs_main);
 
+    /**
+     * Filter for:
+     * (1) wtxids of transactions that were recently rejected by the mempool but are
+     * eligible for reconsideration if submitted with other transactions.
+     * (2) packages (see GetPackageHash) we have already rejected before and should not retry.
+     *
+     * Similar to m_recent_rejects, this filter is used to save bandwidth when e.g. all of our peers
+     * have larger mempools and thus lower minimum feerates than us.
+     *
+     * When a transaction's error is TxValidationResult::TX_RECONSIDERABLE (in a package or by
+     * itself), add its wtxid to this filter. When a package fails for any reason, add the combined
+     * hash to this filter.
+     *
+     * Upon receiving an announcement for a transaction, if it exists in this filter, do not
+     * download the txdata. When considering packages, if it exists in this filter, drop it.
+     *
+     * Reset this filter when the chain tip changes.
+     *
+     * Parameters are picked to be the same as m_recent_rejects, with the same rationale.
+     */
+    CRollingBloomFilter m_recent_rejects_reconsiderable GUARDED_BY(::cs_main){120'000, 0.000'001};
+
     /*
      * Filter for transactions that have been recently confirmed.
      * We use this to avoid requesting transactions that have already been
@@ -2194,7 +2227,7 @@ void PeerManagerImpl::BlockChecked(const CBlock& block, const BlockValidationSta
 //
 
 
-bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid)
+bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid, bool include_reconsiderable)
 {
     if (m_chainman.ActiveChain().Tip()->GetBlockHash() != hashRecentRejectsChainTip) {
         // If the chain tip has changed previously rejected transactions
@@ -2203,12 +2236,15 @@ bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid)
         // txs a second chance.
         hashRecentRejectsChainTip = m_chainman.ActiveChain().Tip()->GetBlockHash();
         m_recent_rejects.reset();
+        m_recent_rejects_reconsiderable.reset();
     }
 
     const uint256& hash = gtxid.GetHash();
 
     if (m_orphanage.HaveTx(gtxid)) return true;
 
+    if (include_reconsiderable && m_recent_rejects_reconsiderable.contains(hash)) return true;
+
     {
         LOCK(m_recent_confirmed_transactions_mutex);
         if (m_recent_confirmed_transactions.contains(hash)) return true;
@@ -3097,7 +3133,14 @@ void PeerManagerImpl::ProcessInvalidTx(NodeId nodeid, const CTransactionRef& ptx
         // See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034
         // for concerns around weakening security of unupgraded nodes
         // if we start doing this too early.
-        m_recent_rejects.insert(ptx->GetWitnessHash().ToUint256());
+        if (state.GetResult() == TxValidationResult::TX_RECONSIDERABLE) {
+            // If the result is TX_RECONSIDERABLE, add it to m_recent_rejects_reconsiderable
+            // because we should not download or submit this transaction by itself again, but may
+            // submit it as part of a package later.
+            m_recent_rejects_reconsiderable.insert(ptx->GetWitnessHash().ToUint256());
+        } else {
+            m_recent_rejects.insert(ptx->GetWitnessHash().ToUint256());
+        }
         m_txrequest.ForgetTxHash(ptx->GetWitnessHash());
         // If the transaction failed for TX_INPUTS_NOT_STANDARD,
         // then we know that the witness was irrelevant to the policy
@@ -4015,7 +4058,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                     return;
                 }
                 const GenTxid gtxid = ToGenTxid(inv);
-                const bool fAlreadyHave = AlreadyHaveTx(gtxid);
+                const bool fAlreadyHave = AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true);
                 LogPrint(BCLog::NET, "got inv: %s  %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId());
 
                 AddKnownTx(*peer, inv.hash);
@@ -4320,7 +4363,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         // already; and an adversary can already relay us old transactions
         // (older than our recency filter) if trying to DoS us, without any need
         // for witness malleation.
-        if (AlreadyHaveTx(GenTxid::Wtxid(wtxid))) {
+        if (AlreadyHaveTx(GenTxid::Wtxid(wtxid), /*include_reconsiderable=*/true)) {
             if (pfrom.HasPermission(NetPermissionFlags::ForceRelay)) {
                 // Always relay transactions received from peers with forcerelay
                 // permission, even if they were already in the mempool, allowing
@@ -4392,7 +4435,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                     // protocol for getting all unconfirmed parents.
                     const auto gtxid{GenTxid::Txid(parent_txid)};
                     AddKnownTx(*peer, parent_txid);
-                    if (!AlreadyHaveTx(gtxid)) AddTxAnnouncement(pfrom, gtxid, current_time);
+                    if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) AddTxAnnouncement(pfrom, gtxid, current_time);
                 }
 
                 if (m_orphanage.AddTx(ptx, pfrom.GetId())) {
@@ -6033,7 +6076,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
                 entry.second.GetHash().ToString(), entry.first);
         }
         for (const GenTxid& gtxid : requestable) {
-            if (!AlreadyHaveTx(gtxid)) {
+            if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) {
                 LogPrint(BCLog::NET, "Requesting %s %s peer=%d\n", gtxid.IsWtxid() ? "wtx" : "tx",
                     gtxid.GetHash().ToString(), pto->GetId());
                 vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.GetHash());

From 87c5c524d63c833cf490c7f2f73d72695ad480df Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Fri, 8 Mar 2024 13:15:47 +0000
Subject: [PATCH 8/9] [p2p] opportunistically accept 1-parent-1-child packages

---
 src/net_processing.cpp | 198 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 196 insertions(+), 2 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 37d9e169e1..da4f99fb99 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -596,6 +596,45 @@ private:
     void ProcessValidTx(NodeId nodeid, const CTransactionRef& tx, const std::list<CTransactionRef>& replaced_transactions)
         EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main);
 
+    /** Handle the results of package validation: calls ProcessValidTx and ProcessInvalidTx for
+     * individual transactions, and caches rejection for the package as a group.
+     * @param[in]   senders     Must contain the nodeids of the peers that provided each transaction
+     *                          in package, in the same order.
+     *   */
+    void ProcessPackageResult(const Package& package, const PackageMempoolAcceptResult& package_result, const std::vector<NodeId>& senders)
+        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main);
+
+    /** A package to validate  */
+    struct PackageToValidate {
+        const Package m_txns;
+        const std::vector<NodeId> m_senders;
+        /** Construct a 1-parent-1-child package. */
+        explicit PackageToValidate(const CTransactionRef& parent,
+                                   const CTransactionRef& child,
+                                   NodeId parent_sender,
+                                   NodeId child_sender) :
+            m_txns{parent, child},
+            m_senders {parent_sender, child_sender}
+        {}
+
+        std::string ToString() const {
+            Assume(m_txns.size() == 2);
+            return strprintf("parent %s (wtxid=%s, sender=%d) + child %s (wtxid=%s, sender=%d)",
+                             m_txns.front()->GetHash().ToString(),
+                             m_txns.front()->GetWitnessHash().ToString(),
+                             m_senders.front(),
+                             m_txns.back()->GetHash().ToString(),
+                             m_txns.back()->GetWitnessHash().ToString(),
+                             m_senders.back());
+        }
+    };
+
+    /** Look for a child of this transaction in the orphanage to form a 1-parent-1-child package,
+     * skipping any combinations that have already been tried. Return the resulting package along with
+     * the senders of its respective transactions, or std::nullopt if no package is found. */
+    std::optional<PackageToValidate> Find1P1CPackage(const CTransactionRef& ptx, NodeId nodeid)
+        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main);
+
     /**
      * Reconsider orphan transactions after a parent has been accepted to the mempool.
      *
@@ -3198,6 +3237,117 @@ void PeerManagerImpl::ProcessValidTx(NodeId nodeid, const CTransactionRef& tx, c
     }
 }
 
+void PeerManagerImpl::ProcessPackageResult(const Package& package, const PackageMempoolAcceptResult& package_result, const std::vector<NodeId>& senders)
+{
+    AssertLockNotHeld(m_peer_mutex);
+    AssertLockHeld(g_msgproc_mutex);
+    AssertLockHeld(cs_main);
+
+    if (package_result.m_state.IsInvalid()) {
+        m_recent_rejects_reconsiderable.insert(GetPackageHash(package));
+    }
+    // We currently only expect to process 1-parent-1-child packages. Remove if this changes.
+    if (!Assume(package.size() == 2)) return;
+
+    // No package results to look through for PCKG_POLICY or PCKG_MEMPOOL_ERROR
+    if (package_result.m_state.GetResult() == PackageValidationResult::PCKG_POLICY ||
+        package_result.m_state.GetResult() == PackageValidationResult::PCKG_MEMPOOL_ERROR) return;
+
+    // Iterate backwards to erase in-package descendants from the orphanage before they become
+    // relevant in AddChildrenToWorkSet.
+    auto package_iter = package.rbegin();
+    auto senders_iter = senders.rbegin();
+    while (package_iter != package.rend()) {
+        const auto& tx = *package_iter;
+        const NodeId nodeid = *senders_iter;
+        const auto it_result{package_result.m_tx_results.find(tx->GetWitnessHash())};
+        if (Assume(it_result != package_result.m_tx_results.end())) {
+            const auto& tx_result = it_result->second;
+            switch (tx_result.m_result_type) {
+                case MempoolAcceptResult::ResultType::VALID:
+                {
+                    Assume(tx_result.m_replaced_transactions.has_value());
+                    std::list<CTransactionRef> empty_replacement_list;
+                    ProcessValidTx(nodeid, tx, tx_result.m_replaced_transactions.value_or(empty_replacement_list));
+                    break;
+                }
+                case MempoolAcceptResult::ResultType::INVALID:
+                case MempoolAcceptResult::ResultType::DIFFERENT_WITNESS:
+                {
+                    // Don't add to vExtraTxnForCompact, as these transactions should have already been
+                    // added there when added to the orphanage or rejected for TX_RECONSIDERABLE.
+                    // This should be updated if package submission is ever used for transactions
+                    // that haven't already been validated before.
+                    ProcessInvalidTx(nodeid, tx, tx_result.m_state, /*maybe_add_extra_compact_tx=*/false);
+                    break;
+                }
+                case MempoolAcceptResult::ResultType::MEMPOOL_ENTRY:
+                {
+                    // AlreadyHaveTx() should be catching transactions that are already in mempool.
+                    Assume(false);
+                    break;
+                }
+            }
+        }
+        package_iter++;
+        senders_iter++;
+    }
+}
+
+std::optional<PeerManagerImpl::PackageToValidate> PeerManagerImpl::Find1P1CPackage(const CTransactionRef& ptx, NodeId nodeid)
+{
+    AssertLockNotHeld(m_peer_mutex);
+    AssertLockHeld(g_msgproc_mutex);
+    AssertLockHeld(cs_main);
+
+    const auto& parent_wtxid{ptx->GetWitnessHash()};
+
+    Assume(m_recent_rejects_reconsiderable.contains(parent_wtxid.ToUint256()));
+
+    // Prefer children from this peer. This helps prevent censorship attempts in which an attacker
+    // sends lots of fake children for the parent, and we (unluckily) keep selecting the fake
+    // children instead of the real one provided by the honest peer.
+    const auto cpfp_candidates_same_peer{m_orphanage.GetChildrenFromSamePeer(ptx, nodeid)};
+
+    // These children should be sorted from newest to oldest. In the (probably uncommon) case
+    // of children that replace each other, this helps us accept the highest feerate (probably the
+    // most recent) one efficiently.
+    for (const auto& child : cpfp_candidates_same_peer) {
+        Package maybe_cpfp_package{ptx, child};
+        if (!m_recent_rejects_reconsiderable.contains(GetPackageHash(maybe_cpfp_package))) {
+            return PeerManagerImpl::PackageToValidate{ptx, child, nodeid, nodeid};
+        }
+    }
+
+    // If no suitable candidate from the same peer is found, also try children that were provided by
+    // a different peer. This is useful because sometimes multiple peers announce both transactions
+    // to us, and we happen to download them from different peers (we wouldn't have known that these
+    // 2 transactions are related). We still want to find 1p1c packages then.
+    //
+    // If we start tracking all announcers of orphans, we can restrict this logic to parent + child
+    // pairs in which both were provided by the same peer, i.e. delete this step.
+    const auto cpfp_candidates_different_peer{m_orphanage.GetChildrenFromDifferentPeer(ptx, nodeid)};
+
+    // Find the first 1p1c that hasn't already been rejected. We randomize the order to not
+    // create a bias that attackers can use to delay package acceptance.
+    //
+    // Create a random permutation of the indices.
+    std::vector<size_t> tx_indices(cpfp_candidates_different_peer.size());
+    std::iota(tx_indices.begin(), tx_indices.end(), 0);
+    Shuffle(tx_indices.begin(), tx_indices.end(), m_rng);
+
+    for (const auto index : tx_indices) {
+        // If we already tried a package and failed for any reason, the combined hash was
+        // cached in m_recent_rejects_reconsiderable.
+        const auto [child_tx, child_sender] = cpfp_candidates_different_peer.at(index);
+        Package maybe_cpfp_package{ptx, child_tx};
+        if (!m_recent_rejects_reconsiderable.contains(GetPackageHash(maybe_cpfp_package))) {
+            return PeerManagerImpl::PackageToValidate{ptx, child_tx, nodeid, child_sender};
+        }
+    }
+    return std::nullopt;
+}
+
 bool PeerManagerImpl::ProcessOrphanTx(Peer& peer)
 {
     AssertLockHeld(g_msgproc_mutex);
@@ -4377,6 +4527,20 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                     RelayTransaction(tx.GetHash(), tx.GetWitnessHash());
                 }
             }
+
+            if (m_recent_rejects_reconsiderable.contains(wtxid)) {
+                // When a transaction is already in m_recent_rejects_reconsiderable, we shouldn't submit
+                // it by itself again. However, look for a matching child in the orphanage, as it is
+                // possible that they succeed as a package.
+                LogPrint(BCLog::TXPACKAGES, "found tx %s (wtxid=%s) in reconsiderable rejects, looking for child in orphanage\n",
+                         txid.ToString(), wtxid.ToString());
+                if (auto package_to_validate{Find1P1CPackage(ptx, pfrom.GetId())}) {
+                    const auto package_result{ProcessNewPackage(m_chainman.ActiveChainstate(), m_mempool, package_to_validate->m_txns, /*test_accept=*/false, /*client_maxfeerate=*/std::nullopt)};
+                    LogDebug(BCLog::TXPACKAGES, "package evaluation for %s: %s\n", package_to_validate->ToString(),
+                             package_result.m_state.IsValid() ? "package accepted" : "package rejected");
+                    ProcessPackageResult(package_to_validate->m_txns, package_result, package_to_validate->m_senders);
+                }
+            }
             // If a tx is detected by m_recent_rejects it is ignored. Because we haven't
             // submitted the tx to our mempool, we won't have computed a DoS
             // score for it or determined exactly why we consider it invalid.
@@ -4418,10 +4582,23 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             }
             std::sort(unique_parents.begin(), unique_parents.end());
             unique_parents.erase(std::unique(unique_parents.begin(), unique_parents.end()), unique_parents.end());
+
+            // Distinguish between parents in m_recent_rejects and m_recent_rejects_reconsiderable.
+            // We can tolerate having up to 1 parent in m_recent_rejects_reconsiderable since we
+            // submit 1p1c packages. However, fail immediately if any are in m_recent_rejects.
+            std::optional<uint256> rejected_parent_reconsiderable;
             for (const uint256& parent_txid : unique_parents) {
                 if (m_recent_rejects.contains(parent_txid)) {
                     fRejectedParents = true;
                     break;
+                } else if (m_recent_rejects_reconsiderable.contains(parent_txid) && !m_mempool.exists(GenTxid::Txid(parent_txid))) {
+                    // More than 1 parent in m_recent_rejects_reconsiderable: 1p1c will not be
+                    // sufficient to accept this package, so just give up here.
+                    if (rejected_parent_reconsiderable.has_value()) {
+                        fRejectedParents = true;
+                        break;
+                    }
+                    rejected_parent_reconsiderable = parent_txid;
                 }
             }
             if (!fRejectedParents) {
@@ -4435,7 +4612,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                     // protocol for getting all unconfirmed parents.
                     const auto gtxid{GenTxid::Txid(parent_txid)};
                     AddKnownTx(*peer, parent_txid);
-                    if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) AddTxAnnouncement(pfrom, gtxid, current_time);
+                    // Exclude m_recent_rejects_reconsiderable: the missing parent may have been
+                    // previously rejected for being too low feerate. This orphan might CPFP it.
+                    if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) AddTxAnnouncement(pfrom, gtxid, current_time);
                 }
 
                 if (m_orphanage.AddTx(ptx, pfrom.GetId())) {
@@ -4467,6 +4646,19 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         if (state.IsInvalid()) {
             ProcessInvalidTx(pfrom.GetId(), ptx, state, /*maybe_add_extra_compact_tx=*/true);
         }
+        // When a transaction fails for TX_RECONSIDERABLE, look for a matching child in the
+        // orphanage, as it is possible that they succeed as a package.
+        if (state.GetResult() == TxValidationResult::TX_RECONSIDERABLE) {
+            LogPrint(BCLog::TXPACKAGES, "tx %s (wtxid=%s) failed but reconsiderable, looking for child in orphanage\n",
+                     txid.ToString(), wtxid.ToString());
+            if (auto package_to_validate{Find1P1CPackage(ptx, pfrom.GetId())}) {
+                const auto package_result{ProcessNewPackage(m_chainman.ActiveChainstate(), m_mempool, package_to_validate->m_txns, /*test_accept=*/false, /*client_maxfeerate=*/std::nullopt)};
+                LogDebug(BCLog::TXPACKAGES, "package evaluation for %s: %s\n", package_to_validate->ToString(),
+                         package_result.m_state.IsValid() ? "package accepted" : "package rejected");
+                ProcessPackageResult(package_to_validate->m_txns, package_result, package_to_validate->m_senders);
+            }
+        }
+
         return;
     }
 
@@ -6076,7 +6268,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
                 entry.second.GetHash().ToString(), entry.first);
         }
         for (const GenTxid& gtxid : requestable) {
-            if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) {
+            // Exclude m_recent_rejects_reconsiderable: we may be requesting a missing parent
+            // that was previously rejected for being too low feerate.
+            if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) {
                 LogPrint(BCLog::NET, "Requesting %s %s peer=%d\n", gtxid.IsWtxid() ? "wtx" : "tx",
                     gtxid.GetHash().ToString(), pto->GetId());
                 vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.GetHash());

From e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Mon, 3 Oct 2022 16:30:42 +0100
Subject: [PATCH 9/9] [functional test] opportunistic 1p1c package submission

---
 test/functional/p2p_1p1c_network.py       | 165 +++++++++
 test/functional/p2p_opportunistic_1p1c.py | 414 ++++++++++++++++++++++
 test/functional/test_runner.py            |   2 +
 3 files changed, 581 insertions(+)
 create mode 100755 test/functional/p2p_1p1c_network.py
 create mode 100755 test/functional/p2p_opportunistic_1p1c.py

diff --git a/test/functional/p2p_1p1c_network.py b/test/functional/p2p_1p1c_network.py
new file mode 100755
index 0000000000..e88c826962
--- /dev/null
+++ b/test/functional/p2p_1p1c_network.py
@@ -0,0 +1,165 @@
+#!/usr/bin/env python3
+# Copyright (c) 2024-present The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""
+Test that 1p1c package submission allows a 1p1c package to propagate in a "network" of nodes. Send
+various packages from different nodes on a network in which some nodes have already received some of
+the transactions (and submitted them to mempool, kept them as orphans or rejected them as
+too-low-feerate transactions). The packages should be received and accepted by all nodes.
+"""
+
+from decimal import Decimal
+from math import ceil
+
+from test_framework.messages import (
+    msg_tx,
+)
+from test_framework.p2p import (
+    P2PInterface,
+)
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    assert_greater_than,
+    fill_mempool,
+)
+from test_framework.wallet import (
+    MiniWallet,
+    MiniWalletMode,
+)
+
+# 1sat/vB feerate denominated in BTC/KvB
+FEERATE_1SAT_VB = Decimal("0.00001000")
+
+class PackageRelayTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = True
+        self.num_nodes = 4
+        # hugely speeds up the test, as it involves multiple hops of tx relay.
+        self.noban_tx_relay = True
+        self.extra_args = [[
+            "-datacarriersize=100000",
+            "-maxmempool=5",
+        ]] * self.num_nodes
+        self.supports_cli = False
+
+    def raise_network_minfee(self):
+        filler_wallet = MiniWallet(self.nodes[0])
+        fill_mempool(self, self.nodes[0], filler_wallet)
+
+        self.log.debug("Wait for the network to sync mempools")
+        self.sync_mempools()
+
+        self.log.debug("Check that all nodes' mempool minimum feerates are above min relay feerate")
+        for node in self.nodes:
+            assert_equal(node.getmempoolinfo()['minrelaytxfee'], FEERATE_1SAT_VB)
+            assert_greater_than(node.getmempoolinfo()['mempoolminfee'], FEERATE_1SAT_VB)
+
+    def create_basic_1p1c(self, wallet):
+        low_fee_parent = wallet.create_self_transfer(fee_rate=FEERATE_1SAT_VB, confirmed_only=True)
+        high_fee_child = wallet.create_self_transfer(utxo_to_spend=low_fee_parent["new_utxo"], fee_rate=999*FEERATE_1SAT_VB)
+        package_hex_basic = [low_fee_parent["hex"], high_fee_child["hex"]]
+        return package_hex_basic, low_fee_parent["tx"], high_fee_child["tx"]
+
+    def create_package_2outs(self, wallet):
+        # First create a tester tx to see the vsize, and then adjust the fees
+        utxo_for_2outs = wallet.get_utxo(confirmed_only=True)
+
+        low_fee_parent_2outs_tester = wallet.create_self_transfer_multi(
+            utxos_to_spend=[utxo_for_2outs],
+            num_outputs=2,
+        )
+
+        # Target 1sat/vB so the number of satoshis is equal to the vsize.
+        # Round up. The goal is to be between min relay feerate and mempool min feerate.
+        fee_2outs = ceil(low_fee_parent_2outs_tester["tx"].get_vsize() / 2)
+
+        low_fee_parent_2outs = wallet.create_self_transfer_multi(
+            utxos_to_spend=[utxo_for_2outs],
+            num_outputs=2,
+            fee_per_output=fee_2outs,
+        )
+
+        # Now create the child
+        high_fee_child_2outs = wallet.create_self_transfer_multi(
+            utxos_to_spend=low_fee_parent_2outs["new_utxos"][::-1],
+            fee_per_output=fee_2outs*100,
+        )
+        return [low_fee_parent_2outs["hex"], high_fee_child_2outs["hex"]], low_fee_parent_2outs["tx"], high_fee_child_2outs["tx"]
+
+    def create_package_2p1c(self, wallet):
+        parent1 = wallet.create_self_transfer(fee_rate=FEERATE_1SAT_VB*10, confirmed_only=True)
+        parent2 = wallet.create_self_transfer(fee_rate=FEERATE_1SAT_VB*20, confirmed_only=True)
+        child = wallet.create_self_transfer_multi(
+            utxos_to_spend=[parent1["new_utxo"], parent2["new_utxo"]],
+            fee_per_output=999*parent1["tx"].get_vsize(),
+        )
+        return [parent1["hex"], parent2["hex"], child["hex"]], parent1["tx"], parent2["tx"], child["tx"]
+
+    def create_packages(self):
+        # 1: Basic 1-parent-1-child package, parent 1sat/vB, child 999sat/vB
+        package_hex_1, parent_1, child_1 = self.create_basic_1p1c(self.wallet)
+
+        # 2: same as 1, parent's txid is the same as its wtxid.
+        package_hex_2, parent_2, child_2 = self.create_basic_1p1c(self.wallet_nonsegwit)
+
+        # 3: 2-parent-1-child package. Both parents are above mempool min feerate. No package submission happens.
+        # We require packages to be child-with-unconfirmed-parents and only allow 1-parent-1-child packages.
+        package_hex_3, parent_31, parent_32, child_3 = self.create_package_2p1c(self.wallet)
+
+        # 4: parent + child package where the child spends 2 different outputs from the parent.
+        package_hex_4, parent_4, child_4 = self.create_package_2outs(self.wallet)
+
+        # Assemble return results
+        packages_to_submit = [package_hex_1, package_hex_2, package_hex_3, package_hex_4]
+        # node0: sender
+        # node1: pre-received the children (orphan)
+        # node3: pre-received the parents (too low fee)
+        # All nodes receive parent_31 ahead of time.
+        txns_to_send = [
+            [],
+            [child_1, child_2, parent_31, child_3, child_4],
+            [parent_31],
+            [parent_1, parent_2, parent_31, parent_4]
+        ]
+
+        return packages_to_submit, txns_to_send
+
+    def run_test(self):
+        self.wallet = MiniWallet(self.nodes[1])
+        self.wallet_nonsegwit = MiniWallet(self.nodes[2], mode=MiniWalletMode.RAW_P2PK)
+        self.generate(self.wallet_nonsegwit, 10)
+        self.generate(self.wallet, 120)
+
+        self.log.info("Fill mempools with large transactions to raise mempool minimum feerates")
+        self.raise_network_minfee()
+
+        # Create the transactions.
+        self.wallet.rescan_utxos(include_mempool=True)
+        packages_to_submit, transactions_to_presend = self.create_packages()
+
+        self.peers = [self.nodes[i].add_p2p_connection(P2PInterface()) for i in range(self.num_nodes)]
+
+        self.log.info("Pre-send some transactions to nodes")
+        for (i, peer) in enumerate(self.peers):
+            for tx in transactions_to_presend[i]:
+                peer.send_and_ping(msg_tx(tx))
+            # This disconnect removes any sent orphans from the orphanage (EraseForPeer) and times
+            # out the in-flight requests.  It is currently required for the test to pass right now,
+            # because the node will not reconsider an orphan tx and will not (re)try requesting
+            # orphan parents from multiple peers if the first one didn't respond.
+            # TODO: remove this in the future if the node tries orphan resolution with multiple peers.
+            peer.peer_disconnect()
+
+        self.log.info("Submit full packages to node0")
+        for package_hex in packages_to_submit:
+            submitpackage_result = self.nodes[0].submitpackage(package_hex)
+            assert_equal(submitpackage_result["package_msg"], "success")
+
+        self.log.info("Wait for mempools to sync")
+        self.sync_mempools(timeout=20)
+
+
+if __name__ == '__main__':
+    PackageRelayTest().main()
diff --git a/test/functional/p2p_opportunistic_1p1c.py b/test/functional/p2p_opportunistic_1p1c.py
new file mode 100755
index 0000000000..e07acd5481
--- /dev/null
+++ b/test/functional/p2p_opportunistic_1p1c.py
@@ -0,0 +1,414 @@
+#!/usr/bin/env python3
+# Copyright (c) 2024-present The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""
+Test opportunistic 1p1c package submission logic.
+"""
+
+from decimal import Decimal
+import time
+from test_framework.messages import (
+    CInv,
+    CTxInWitness,
+    MAX_BIP125_RBF_SEQUENCE,
+    MSG_WTX,
+    msg_inv,
+    msg_tx,
+    tx_from_hex,
+)
+from test_framework.p2p import (
+    P2PInterface,
+)
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    assert_greater_than,
+    fill_mempool,
+)
+from test_framework.wallet import (
+    MiniWallet,
+    MiniWalletMode,
+)
+
+# 1sat/vB feerate denominated in BTC/KvB
+FEERATE_1SAT_VB = Decimal("0.00001000")
+# Number of seconds to wait to ensure no getdata is received
+GETDATA_WAIT = 60
+
+def cleanup(func):
+    def wrapper(self, *args, **kwargs):
+        try:
+            func(self, *args, **kwargs)
+        finally:
+            self.nodes[0].disconnect_p2ps()
+            # Do not clear the node's mempool, as each test requires mempool min feerate > min
+            # relay feerate. However, do check that this is the case.
+            assert self.nodes[0].getmempoolinfo()["mempoolminfee"] > self.nodes[0].getnetworkinfo()["relayfee"]
+            # Ensure we do not try to spend the same UTXOs in subsequent tests, as they will look like RBF attempts.
+            self.wallet.rescan_utxos(include_mempool=True)
+
+            # Resets if mocktime was used
+            self.nodes[0].setmocktime(0)
+    return wrapper
+
+class PackageRelayTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = True
+        self.num_nodes = 1
+        self.extra_args = [[
+            "-datacarriersize=100000",
+            "-maxmempool=5",
+        ]]
+        self.supports_cli = False
+
+    def create_tx_below_mempoolminfee(self, wallet):
+        """Create a 1-input 1sat/vB transaction using a confirmed UTXO. Decrement and use
+        self.sequence so that subsequent calls to this function result in unique transactions."""
+
+        self.sequence -= 1
+        assert_greater_than(self.nodes[0].getmempoolinfo()["mempoolminfee"], FEERATE_1SAT_VB)
+
+        return wallet.create_self_transfer(fee_rate=FEERATE_1SAT_VB, sequence=self.sequence, confirmed_only=True)
+
+    @cleanup
+    def test_basic_child_then_parent(self):
+        node = self.nodes[0]
+        self.log.info("Check that opportunistic 1p1c logic works when child is received before parent")
+
+        low_fee_parent = self.create_tx_below_mempoolminfee(self.wallet)
+        high_fee_child = self.wallet.create_self_transfer(utxo_to_spend=low_fee_parent["new_utxo"], fee_rate=20*FEERATE_1SAT_VB)
+
+        peer_sender = node.add_p2p_connection(P2PInterface())
+
+        # 1. Child is received first (perhaps the low feerate parent didn't meet feefilter or the requests were sent to different nodes). It is missing an input.
+        high_child_wtxid_int = int(high_fee_child["tx"].getwtxid(), 16)
+        peer_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=high_child_wtxid_int)]))
+        peer_sender.wait_for_getdata([high_child_wtxid_int])
+        peer_sender.send_and_ping(msg_tx(high_fee_child["tx"]))
+
+        # 2. Node requests the missing parent by txid.
+        parent_txid_int = int(low_fee_parent["txid"], 16)
+        peer_sender.wait_for_getdata([parent_txid_int])
+
+        # 3. Sender relays the parent. Parent+Child are evaluated as a package and accepted.
+        peer_sender.send_and_ping(msg_tx(low_fee_parent["tx"]))
+
+        # 4. Both transactions should now be in mempool.
+        node_mempool = node.getrawmempool()
+        assert low_fee_parent["txid"] in node_mempool
+        assert high_fee_child["txid"] in node_mempool
+
+        node.disconnect_p2ps()
+
+    @cleanup
+    def test_basic_parent_then_child(self, wallet):
+        node = self.nodes[0]
+        low_fee_parent = self.create_tx_below_mempoolminfee(wallet)
+        high_fee_child = wallet.create_self_transfer(utxo_to_spend=low_fee_parent["new_utxo"], fee_rate=20*FEERATE_1SAT_VB)
+
+        peer_sender = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=1, connection_type="outbound-full-relay")
+        peer_ignored = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=2, connection_type="outbound-full-relay")
+
+        # 1. Parent is relayed first. It is too low feerate.
+        parent_wtxid_int = int(low_fee_parent["tx"].getwtxid(), 16)
+        peer_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=parent_wtxid_int)]))
+        peer_sender.wait_for_getdata([parent_wtxid_int])
+        peer_sender.send_and_ping(msg_tx(low_fee_parent["tx"]))
+        assert low_fee_parent["txid"] not in node.getrawmempool()
+
+        # Send again from peer_ignored, check that it is ignored
+        peer_ignored.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=parent_wtxid_int)]))
+        assert "getdata" not in peer_ignored.last_message
+
+        # 2. Child is relayed next. It is missing an input.
+        high_child_wtxid_int = int(high_fee_child["tx"].getwtxid(), 16)
+        peer_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=high_child_wtxid_int)]))
+        peer_sender.wait_for_getdata([high_child_wtxid_int])
+        peer_sender.send_and_ping(msg_tx(high_fee_child["tx"]))
+
+        # 3. Node requests the missing parent by txid.
+        # It should do so even if it has previously rejected that parent for being too low feerate.
+        parent_txid_int = int(low_fee_parent["txid"], 16)
+        peer_sender.wait_for_getdata([parent_txid_int])
+
+        # 4. Sender re-relays the parent. Parent+Child are evaluated as a package and accepted.
+        peer_sender.send_and_ping(msg_tx(low_fee_parent["tx"]))
+
+        # 5. Both transactions should now be in mempool.
+        node_mempool = node.getrawmempool()
+        assert low_fee_parent["txid"] in node_mempool
+        assert high_fee_child["txid"] in node_mempool
+
+    @cleanup
+    def test_low_and_high_child(self, wallet):
+        node = self.nodes[0]
+        low_fee_parent = self.create_tx_below_mempoolminfee(wallet)
+        # This feerate is above mempoolminfee, but not enough to also bump the low feerate parent.
+        feerate_just_above = node.getmempoolinfo()["mempoolminfee"]
+        med_fee_child = wallet.create_self_transfer(utxo_to_spend=low_fee_parent["new_utxo"], fee_rate=feerate_just_above)
+        high_fee_child = wallet.create_self_transfer(utxo_to_spend=low_fee_parent["new_utxo"], fee_rate=999*FEERATE_1SAT_VB)
+
+        peer_sender = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=1, connection_type="outbound-full-relay")
+        peer_ignored = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=2, connection_type="outbound-full-relay")
+
+        self.log.info("Check that tx caches low fee parent + low fee child package rejections")
+
+        # 1. Send parent, rejected for being low feerate.
+        parent_wtxid_int = int(low_fee_parent["tx"].getwtxid(), 16)
+        peer_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=parent_wtxid_int)]))
+        peer_sender.wait_for_getdata([parent_wtxid_int])
+        peer_sender.send_and_ping(msg_tx(low_fee_parent["tx"]))
+        assert low_fee_parent["txid"] not in node.getrawmempool()
+
+        # Send again from peer_ignored, check that it is ignored
+        peer_ignored.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=parent_wtxid_int)]))
+        assert "getdata" not in peer_ignored.last_message
+
+        # 2. Send an (orphan) child that has a higher feerate, but not enough to bump the parent.
+        med_child_wtxid_int = int(med_fee_child["tx"].getwtxid(), 16)
+        peer_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=med_child_wtxid_int)]))
+        peer_sender.wait_for_getdata([med_child_wtxid_int])
+        peer_sender.send_and_ping(msg_tx(med_fee_child["tx"]))
+
+        # 3. Node requests the orphan's missing parent.
+        parent_txid_int = int(low_fee_parent["txid"], 16)
+        peer_sender.wait_for_getdata([parent_txid_int])
+
+        # 4. The low parent + low child are submitted as a package. They are not accepted due to low package feerate.
+        peer_sender.send_and_ping(msg_tx(low_fee_parent["tx"]))
+
+        assert low_fee_parent["txid"] not in node.getrawmempool()
+        assert med_fee_child["txid"] not in node.getrawmempool()
+
+        # If peer_ignored announces the low feerate child, it should be ignored
+        peer_ignored.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=med_child_wtxid_int)]))
+        assert "getdata" not in peer_ignored.last_message
+        # If either peer sends the parent again, package evaluation should not be attempted
+        peer_sender.send_and_ping(msg_tx(low_fee_parent["tx"]))
+        peer_ignored.send_and_ping(msg_tx(low_fee_parent["tx"]))
+
+        assert low_fee_parent["txid"] not in node.getrawmempool()
+        assert med_fee_child["txid"] not in node.getrawmempool()
+
+        # 5. Send the high feerate (orphan) child
+        high_child_wtxid_int = int(high_fee_child["tx"].getwtxid(), 16)
+        peer_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=high_child_wtxid_int)]))
+        peer_sender.wait_for_getdata([high_child_wtxid_int])
+        peer_sender.send_and_ping(msg_tx(high_fee_child["tx"]))
+
+        # 6. Node requests the orphan's parent, even though it has already been rejected, both by
+        # itself and with a child. This is necessary, otherwise high_fee_child can be censored.
+        parent_txid_int = int(low_fee_parent["txid"], 16)
+        peer_sender.wait_for_getdata([parent_txid_int])
+
+        # 7. The low feerate parent + high feerate child are submitted as a package.
+        peer_sender.send_and_ping(msg_tx(low_fee_parent["tx"]))
+
+        # 8. Both transactions should now be in mempool
+        node_mempool = node.getrawmempool()
+        assert low_fee_parent["txid"] in node_mempool
+        assert high_fee_child["txid"] in node_mempool
+        assert med_fee_child["txid"] not in node_mempool
+
+    @cleanup
+    def test_orphan_consensus_failure(self):
+        self.log.info("Check opportunistic 1p1c logic with consensus-invalid orphan causes disconnect of the correct peer")
+        node = self.nodes[0]
+        low_fee_parent = self.create_tx_below_mempoolminfee(self.wallet)
+        coin = low_fee_parent["new_utxo"]
+        address = node.get_deterministic_priv_key().address
+        # Create raw transaction spending the parent, but with no signature (a consensus error).
+        hex_orphan_no_sig = node.createrawtransaction([{"txid": coin["txid"], "vout": coin["vout"]}], {address : coin["value"] - Decimal("0.0001")})
+        tx_orphan_bad_wit = tx_from_hex(hex_orphan_no_sig)
+        tx_orphan_bad_wit.wit.vtxinwit.append(CTxInWitness())
+        tx_orphan_bad_wit.wit.vtxinwit[0].scriptWitness.stack = [b'garbage']
+
+        bad_orphan_sender = node.add_p2p_connection(P2PInterface())
+        parent_sender = node.add_p2p_connection(P2PInterface())
+
+        # 1. Child is received first. It is missing an input.
+        child_wtxid_int = int(tx_orphan_bad_wit.getwtxid(), 16)
+        bad_orphan_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=child_wtxid_int)]))
+        bad_orphan_sender.wait_for_getdata([child_wtxid_int])
+        bad_orphan_sender.send_and_ping(msg_tx(tx_orphan_bad_wit))
+
+        # 2. Node requests the missing parent by txid.
+        parent_txid_int = int(low_fee_parent["txid"], 16)
+        bad_orphan_sender.wait_for_getdata([parent_txid_int])
+
+        # 3. A different peer relays the parent. Parent+Child are evaluated as a package and rejected.
+        parent_sender.send_message(msg_tx(low_fee_parent["tx"]))
+
+        # 4. Transactions should not be in mempool.
+        node_mempool = node.getrawmempool()
+        assert low_fee_parent["txid"] not in node_mempool
+        assert tx_orphan_bad_wit.rehash() not in node_mempool
+
+        # 5. Peer that sent a consensus-invalid transaction should be disconnected.
+        bad_orphan_sender.wait_for_disconnect()
+
+        # The peer that didn't provide the orphan should not be disconnected.
+        parent_sender.sync_with_ping()
+
+    @cleanup
+    def test_parent_consensus_failure(self):
+        self.log.info("Check opportunistic 1p1c logic with consensus-invalid parent causes disconnect of the correct peer")
+        node = self.nodes[0]
+        low_fee_parent = self.create_tx_below_mempoolminfee(self.wallet)
+        high_fee_child = self.wallet.create_self_transfer(utxo_to_spend=low_fee_parent["new_utxo"], fee_rate=999*FEERATE_1SAT_VB)
+
+        # Create invalid version of parent with a bad signature.
+        tx_parent_bad_wit = tx_from_hex(low_fee_parent["hex"])
+        tx_parent_bad_wit.wit.vtxinwit.append(CTxInWitness())
+        tx_parent_bad_wit.wit.vtxinwit[0].scriptWitness.stack = [b'garbage']
+
+        package_sender = node.add_p2p_connection(P2PInterface())
+        fake_parent_sender = node.add_p2p_connection(P2PInterface())
+
+        # 1. Child is received first. It is missing an input.
+        child_wtxid_int = int(high_fee_child["tx"].getwtxid(), 16)
+        package_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=child_wtxid_int)]))
+        package_sender.wait_for_getdata([child_wtxid_int])
+        package_sender.send_and_ping(msg_tx(high_fee_child["tx"]))
+
+        # 2. Node requests the missing parent by txid.
+        parent_txid_int = int(tx_parent_bad_wit.rehash(), 16)
+        package_sender.wait_for_getdata([parent_txid_int])
+
+        # 3. A different node relays the parent. The parent is first evaluated by itself and
+        # rejected for being too low feerate. Then it is evaluated as a package and, after passing
+        # feerate checks, rejected for having a bad signature (consensus error).
+        fake_parent_sender.send_message(msg_tx(tx_parent_bad_wit))
+
+        # 4. Transactions should not be in mempool.
+        node_mempool = node.getrawmempool()
+        assert tx_parent_bad_wit.rehash() not in node_mempool
+        assert high_fee_child["txid"] not in node_mempool
+
+        # 5. Peer sent a consensus-invalid transaction.
+        fake_parent_sender.wait_for_disconnect()
+
+        self.log.info("Check that fake parent does not cause orphan to be deleted and real package can still be submitted")
+        # 6. Child-sending should not have been punished and the orphan should remain in orphanage.
+        # It can send the "real" parent transaction, and the package is accepted.
+        parent_wtxid_int = int(low_fee_parent["tx"].getwtxid(), 16)
+        package_sender.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=parent_wtxid_int)]))
+        package_sender.wait_for_getdata([parent_wtxid_int])
+        package_sender.send_and_ping(msg_tx(low_fee_parent["tx"]))
+
+        node_mempool = node.getrawmempool()
+        assert low_fee_parent["txid"] in node_mempool
+        assert high_fee_child["txid"] in node_mempool
+
+    @cleanup
+    def test_multiple_parents(self):
+        self.log.info("Check that node does not request more than 1 previously-rejected low feerate parent")
+
+        node = self.nodes[0]
+        node.setmocktime(int(time.time()))
+
+        # 2-parent-1-child package where both parents are below mempool min feerate
+        parent_low_1 = self.create_tx_below_mempoolminfee(self.wallet_nonsegwit)
+        parent_low_2 = self.create_tx_below_mempoolminfee(self.wallet_nonsegwit)
+        child_bumping = self.wallet_nonsegwit.create_self_transfer_multi(
+            utxos_to_spend=[parent_low_1["new_utxo"], parent_low_2["new_utxo"]],
+            fee_per_output=999*parent_low_1["tx"].get_vsize(),
+        )
+
+        peer_sender = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=1, connection_type="outbound-full-relay")
+
+        # 1. Send both parents. Each should be rejected for being too low feerate.
+        # Send unsolicited so that we can later check that no "getdata" was ever received.
+        peer_sender.send_and_ping(msg_tx(parent_low_1["tx"]))
+        peer_sender.send_and_ping(msg_tx(parent_low_2["tx"]))
+
+        # parent_low_1 and parent_low_2 are rejected for being low feerate.
+        assert parent_low_1["txid"] not in node.getrawmempool()
+        assert parent_low_2["txid"] not in node.getrawmempool()
+
+        # 2. Send child.
+        peer_sender.send_and_ping(msg_tx(child_bumping["tx"]))
+
+        # 3. Node should not request any parents, as it should recognize that it will not accept
+        # multi-parent-1-child packages.
+        node.bumpmocktime(GETDATA_WAIT)
+        peer_sender.sync_with_ping()
+        assert "getdata" not in peer_sender.last_message
+
+    @cleanup
+    def test_other_parent_in_mempool(self):
+        self.log.info("Check opportunistic 1p1c fails if child already has another parent in mempool")
+        node = self.nodes[0]
+
+        # This parent needs CPFP
+        parent_low = self.create_tx_below_mempoolminfee(self.wallet)
+        # This parent does not need CPFP and can be submitted alone ahead of time
+        parent_high = self.wallet.create_self_transfer(fee_rate=FEERATE_1SAT_VB*10, confirmed_only=True)
+        child = self.wallet.create_self_transfer_multi(
+            utxos_to_spend=[parent_high["new_utxo"], parent_low["new_utxo"]],
+            fee_per_output=999*parent_low["tx"].get_vsize(),
+        )
+
+        peer_sender = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=1, connection_type="outbound-full-relay")
+
+        # 1. Send first parent which will be accepted.
+        peer_sender.send_and_ping(msg_tx(parent_high["tx"]))
+        assert parent_high["txid"] in node.getrawmempool()
+
+        # 2. Send child.
+        peer_sender.send_and_ping(msg_tx(child["tx"]))
+
+        # 3. Node requests parent_low. However, 1p1c fails because package-not-child-with-unconfirmed-parents
+        parent_low_txid_int = int(parent_low["txid"], 16)
+        peer_sender.wait_for_getdata([parent_low_txid_int])
+        peer_sender.send_and_ping(msg_tx(parent_low["tx"]))
+
+        node_mempool = node.getrawmempool()
+        assert parent_high["txid"] in node_mempool
+        assert parent_low["txid"] not in node_mempool
+        assert child["txid"] not in node_mempool
+
+        # Same error if submitted through submitpackage without parent_high
+        package_hex_missing_parent = [parent_low["hex"], child["hex"]]
+        result_missing_parent = node.submitpackage(package_hex_missing_parent)
+        assert_equal(result_missing_parent["package_msg"], "package-not-child-with-unconfirmed-parents")
+
+    def run_test(self):
+        node = self.nodes[0]
+        # To avoid creating transactions with the same txid (can happen if we set the same feerate
+        # and reuse the same input as a previous transaction that wasn't successfully submitted),
+        # we give each subtest a different nSequence for its transactions.
+        self.sequence = MAX_BIP125_RBF_SEQUENCE
+
+        self.wallet = MiniWallet(node)
+        self.wallet_nonsegwit = MiniWallet(node, mode=MiniWalletMode.RAW_P2PK)
+        self.generate(self.wallet_nonsegwit, 10)
+        self.generate(self.wallet, 20)
+
+        filler_wallet = MiniWallet(node)
+        fill_mempool(self, node, filler_wallet)
+
+        self.log.info("Check opportunistic 1p1c logic when parent (txid != wtxid) is received before child")
+        self.test_basic_parent_then_child(self.wallet)
+
+        self.log.info("Check opportunistic 1p1c logic when parent (txid == wtxid) is received before child")
+        self.test_basic_parent_then_child(self.wallet_nonsegwit)
+
+        self.log.info("Check opportunistic 1p1c logic when child is received before parent")
+        self.test_basic_child_then_parent()
+
+        self.log.info("Check opportunistic 1p1c logic when 2 candidate children exist (parent txid != wtxid)")
+        self.test_low_and_high_child(self.wallet)
+
+        self.log.info("Check opportunistic 1p1c logic when 2 candidate children exist (parent txid == wtxid)")
+        self.test_low_and_high_child(self.wallet_nonsegwit)
+
+        self.test_orphan_consensus_failure()
+        self.test_parent_consensus_failure()
+        self.test_multiple_parents()
+        self.test_other_parent_in_mempool()
+
+
+if __name__ == '__main__':
+    PackageRelayTest().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 3f6e47d410..629c9d025a 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -199,6 +199,8 @@ BASE_SCRIPTS = [
     'wallet_txn_clone.py --segwit',
     'rpc_getchaintips.py',
     'rpc_misc.py',
+    'p2p_1p1c_network.py',
+    'p2p_opportunistic_1p1c.py',
     'interface_rest.py',
     'mempool_spend_coinbase.py',
     'wallet_avoid_mixing_output_types.py --descriptors',