Merge 86d7135e36 into 66aa6a47bd

7c123c08dd  miner: add package feerate vector to CBlockTemplate (ismaelsadeeq)

Pull request description:

  This PR enables `BlockAssembler` to add all selected packages' fee and virtual size to a vector, and then return the vector as a member of `CBlockTemplate` struct.

  This PR is the first step in the https://github.com/bitcoin/bitcoin/issues/30392 project.

  The packages' vsize and fee are used in #30157 to select a percentile fee rate of the top block in the mempool.

ACKs for top commit:
  rkrux:
    tACK 7c123c08dd
  ryanofsky:
    Code review ACK 7c123c08dd. Changes since last review are rebasing due to a test conflict, giving the new field a better name and description, resolving the test conflict, and renaming a lot of test variables. The actual code change is still one-line change.
  glozow:
    reACK 7c123c08dd

Tree-SHA512: 767b0b3d4273cf1589fd2068d729a66c7414c0f9574b15989fbe293f8c85cd6c641dd783cde55bfabab32cd047d7d8a071d6897b06ed4295c0d071e588de0861

src/net_processing.cpp

@@ -2978,7 +2978,7 @@ std::optional<node::PackageToValidate> PeerManagerImpl::ProcessInvalidTx(NodeId
     if (add_extra_compact_tx && RecursiveDynamicUsage(*ptx) < 100000) {
         AddToCompactExtraTransactions(ptx);
     }
-    for (const uint256& parent_txid : unique_parents) {
+    for (const Txid& parent_txid : unique_parents) {
         if (peer) AddKnownTx(*peer, parent_txid);
     }
 
@@ -3934,7 +3934,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                 AddKnownTx(*peer, inv.hash);
 
                 if (!m_chainman.IsInitialBlockDownload()) {
-                    const bool fAlreadyHave{m_txdownloadman.AddTxAnnouncement(pfrom.GetId(), gtxid, current_time, /*p2p_inv=*/true)};
+                    const bool fAlreadyHave{m_txdownloadman.AddTxAnnouncement(pfrom.GetId(), gtxid, current_time)};
                     LogDebug(BCLog::NET, "got inv: %s  %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId());
                 }
             } else {

src/node/miner.cpp

@@ -421,6 +421,7 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda
         }
 
         ++nPackagesSelected;
+        pblocktemplate->m_package_feerates.emplace_back(packageFees, static_cast<int32_t>(packageSize));
 
         // Update transactions that depend on each of these
         nDescendantsUpdated += UpdatePackagesForAdded(mempool, ancestors, mapModifiedTx);

src/node/miner.h

@@ -10,6 +10,7 @@
 #include <policy/policy.h>
 #include <primitives/block.h>
 #include <txmempool.h>
+#include <util/feefrac.h>
 
 #include <memory>
 #include <optional>
@@ -39,6 +40,9 @@ struct CBlockTemplate
     std::vector<CAmount> vTxFees;
     std::vector<int64_t> vTxSigOpsCost;
     std::vector<unsigned char> vchCoinbaseCommitment;
+    /* A vector of package fee rates, ordered by the sequence in which
+     * packages are selected for inclusion in the block template.*/
+    std::vector<FeeFrac> m_package_feerates;
 };
 
 // Container for tracking updates to ancestor feerate as we include (parent)

src/node/txdownloadman.h

@@ -92,7 +92,7 @@ struct PackageToValidate {
 struct RejectedTxTodo
 {
     bool m_should_add_extra_compact_tx;
-    std::vector<uint256> m_unique_parents;
+    std::vector<Txid> m_unique_parents;
     std::optional<PackageToValidate> m_package_to_validate;
 };
 
@@ -136,9 +136,8 @@ public:
 
     /** Consider adding this tx hash to txrequest. Should be called whenever a new inv has been received.
      * Also called internally when a transaction is missing parents so that we can request them.
-     * @param[in] p2p_inv     When true, only add this announcement if we don't already have the tx.
      * Returns true if this was a dropped inv (p2p_inv=true and we already have the tx), false otherwise. */
-    bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv);
+    bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now);
 
     /** Get getdata requests to send. */
     std::vector<GenTxid> GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time);

src/node/txdownloadman_impl.cpp

@@ -39,9 +39,9 @@ void TxDownloadManager::DisconnectedPeer(NodeId nodeid)
 {
     m_impl->DisconnectedPeer(nodeid);
 }
-bool TxDownloadManager::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv)
+bool TxDownloadManager::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now)
 {
-    return m_impl->AddTxAnnouncement(peer, gtxid, now, p2p_inv);
+    return m_impl->AddTxAnnouncement(peer, gtxid, now);
 }
 std::vector<GenTxid> TxDownloadManager::GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time)
 {
@@ -172,12 +172,39 @@ void TxDownloadManagerImpl::DisconnectedPeer(NodeId nodeid)
 
 }
 
-bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv)
+bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now)
 {
+    // If this is an orphan we are trying to resolve, consider this peer as a orphan resolution candidate instead.
+    // - is wtxid matching something in orphanage
+    // - exists in orphanage
+    // - peer can be an orphan resolution candidate
+    if (gtxid.IsWtxid()) {
+        if (auto orphan_tx{m_orphanage.GetTx(Wtxid::FromUint256(gtxid.GetHash()))}) {
+            auto unique_parents{GetUniqueParents(*orphan_tx)};
+            std::erase_if(unique_parents, [&](const auto& txid){
+                return AlreadyHaveTx(GenTxid::Txid(txid), /*include_reconsiderable=*/false);
+            });
+
+            if (unique_parents.empty()) return true;
+
+            if (auto delay{OrphanResolutionCandidate(peer, Wtxid::FromUint256(gtxid.GetHash()), unique_parents.size())}) {
+                m_orphanage.AddAnnouncer(Wtxid::FromUint256(gtxid.GetHash()), peer);
+
+                const auto& info = m_peer_info.at(peer).m_connection_info;
+                for (const auto& parent_txid : unique_parents) {
+                    m_txrequest.ReceivedInv(peer, GenTxid::Txid(parent_txid), info.m_preferred, now + *delay);
+                }
+
+                LogDebug(BCLog::TXPACKAGES, "added peer=%d as a candidate for resolving orphan %s\n", peer, gtxid.GetHash().ToString());
+            }
+
+            // Return even if the peer isn't an orphan resolution candidate. This would be caught by AlreadyHaveTx.
+            return true;
+        }
+    }
+
     // If this is an inv received from a peer and we already have it, we can drop it.
-    // If this is a request for the parent of an orphan, we don't drop transactions that we already have. In particular,
-    // we *do* want to request parents that are in m_lazy_recent_rejects_reconsiderable, since they can be CPFP'd.
-    if (p2p_inv && AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) return true;
+    if (AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) return true;
 
     auto it = m_peer_info.find(peer);
     if (it == m_peer_info.end()) return false;
@@ -204,6 +231,36 @@ bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid,
     return false;
 }
 
+std::optional<std::chrono::seconds> TxDownloadManagerImpl::OrphanResolutionCandidate(NodeId nodeid, const Wtxid& orphan_wtxid, size_t num_parents)
+{
+    if (m_peer_info.count(nodeid) == 0) return std::nullopt;
+    if (m_orphanage.HaveTxFromPeer(orphan_wtxid, nodeid)) return std::nullopt;
+
+    const auto& peer_entry = m_peer_info.at(nodeid);
+    const auto& info = peer_entry.m_connection_info;
+    // TODO: add delays and limits based on the amount of orphan resolution we are already doing
+    // with this peer, how much they are using the orphanage, etc.
+    if (!info.m_relay_permissions) {
+        // This mirrors the delaying and dropping behavior in AddTxAnnouncement in order to preserve
+        // existing behavior: drop if we are tracking too many invs for this peer already. Each
+        // orphan resolution involves at least 1 transaction request which may or may not be
+        // currently tracked in m_txrequest, so we include that in the count.
+        if (m_txrequest.Count(nodeid) + num_parents > MAX_PEER_TX_ANNOUNCEMENTS) return std::nullopt;
+    }
+
+    std::chrono::seconds delay{0s};
+    if (!info.m_preferred) delay += NONPREF_PEER_TX_DELAY;
+    // The orphan wtxid is used, but resolution entails requesting the parents by txid. Sometimes
+    // parent and child are announced and thus requested around the same time, and we happen to
+    // receive child sooner. Waiting a few seconds may allow us to cancel the orphan resolution
+    // request if the parent arrives in that time.
+    if (m_num_wtxid_peers > 0) delay += TXID_RELAY_DELAY;
+    const bool overloaded = !info.m_relay_permissions && m_txrequest.CountInFlight(nodeid) >= MAX_PEER_TX_REQUEST_IN_FLIGHT;
+    if (overloaded) delay += OVERLOADED_PEER_TX_DELAY;
+
+    return delay;
+}
+
 std::vector<GenTxid> TxDownloadManagerImpl::GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time)
 {
     std::vector<GenTxid> requests;
@@ -243,9 +300,11 @@ std::optional<PackageToValidate> TxDownloadManagerImpl::Find1P1CPackage(const CT
 
     Assume(RecentRejectsReconsiderableFilter().contains(parent_wtxid.ToUint256()));
 
-    // Prefer children from this peer. This helps prevent censorship attempts in which an attacker
+    // Only consider 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.
+    // children instead of the real one provided by the honest peer. Since we track all announcers
+    // of an orphan, this does not exclude parent + orphan pairs that we happened to request from
+    // different peers.
     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
@@ -258,34 +317,6 @@ std::optional<PackageToValidate> TxDownloadManagerImpl::Find1P1CPackage(const CT
             return 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);
-    std::shuffle(tx_indices.begin(), tx_indices.end(), m_opts.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_lazy_recent_rejects_reconsiderable.
-        const auto [child_tx, child_sender] = cpfp_candidates_different_peer.at(index);
-        Package maybe_cpfp_package{ptx, child_tx};
-        if (!RecentRejectsReconsiderableFilter().contains(GetPackageHash(maybe_cpfp_package)) &&
-            !RecentRejectsFilter().contains(child_tx->GetHash().ToUint256())) {
-            return PackageToValidate{ptx, child_tx, nodeid, child_sender};
-        }
-    }
     return std::nullopt;
 }
 
@@ -301,6 +332,21 @@ void TxDownloadManagerImpl::MempoolAcceptedTx(const CTransactionRef& tx)
     m_orphanage.EraseTx(tx->GetWitnessHash());
 }
 
+std::vector<Txid> TxDownloadManagerImpl::GetUniqueParents(const CTransaction& tx)
+{
+    std::vector<Txid> unique_parents;
+    unique_parents.reserve(tx.vin.size());
+    for (const CTxIn& txin : tx.vin) {
+        // We start with all parents, and then remove duplicates below.
+        unique_parents.push_back(txin.prevout.hash);
+    }
+
+    std::sort(unique_parents.begin(), unique_parents.end());
+    unique_parents.erase(std::unique(unique_parents.begin(), unique_parents.end()), unique_parents.end());
+
+    return unique_parents;
+}
+
 node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransactionRef& ptx, const TxValidationState& state, NodeId nodeid, bool first_time_failure)
 {
     const CTransaction& tx{*ptx};
@@ -308,7 +354,7 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
     // Whether we should call AddToCompactExtraTransactions at the end
     bool add_extra_compact_tx{first_time_failure};
     // Hashes to pass to AddKnownTx later
-    std::vector<uint256> unique_parents;
+    std::vector<Txid> unique_parents;
     // Populated if failure is reconsiderable and eligible package is found.
     std::optional<node::PackageToValidate> package_to_validate;
 
@@ -320,13 +366,7 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
 
             // Deduplicate parent txids, so that we don't have to loop over
             // the same parent txid more than once down below.
-            unique_parents.reserve(tx.vin.size());
-            for (const CTxIn& txin : tx.vin) {
-                // We start with all parents, and then remove duplicates below.
-                unique_parents.push_back(txin.prevout.hash);
-            }
-            std::sort(unique_parents.begin(), unique_parents.end());
-            unique_parents.erase(std::unique(unique_parents.begin(), unique_parents.end()), unique_parents.end());
+            unique_parents = GetUniqueParents(tx);
 
             // Distinguish between parents in m_lazy_recent_rejects and m_lazy_recent_rejects_reconsiderable.
             // We can tolerate having up to 1 parent in m_lazy_recent_rejects_reconsiderable since we
@@ -348,30 +388,48 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
                 }
             }
             if (!fRejectedParents) {
-                const auto current_time{GetTime<std::chrono::microseconds>()};
+                // Filter parents that we already have.
+                // Exclude m_lazy_recent_rejects_reconsiderable: the missing parent may have been
+                // previously rejected for being too low feerate. This orphan might CPFP it.
+                std::erase_if(unique_parents, [&](const auto& txid){
+                    return AlreadyHaveTx(GenTxid::Txid(txid), /*include_reconsiderable=*/false);
+                });
+                const auto now{GetTime<std::chrono::microseconds>()};
+                const auto& wtxid = ptx->GetWitnessHash();
+                // Potentially flip add_extra_compact_tx to false if tx is already in orphanage, which
+                // means it was already added to vExtraTxnForCompact.
+                add_extra_compact_tx &= !m_orphanage.HaveTx(wtxid);
 
-                for (const uint256& parent_txid : unique_parents) {
-                    // Here, we only have the txid (and not wtxid) of the
-                    // inputs, so we only request in txid mode, even for
-                    // wtxidrelay peers.
-                    // Eventually we should replace this with an improved
-                    // protocol for getting all unconfirmed parents.
-                    const auto gtxid{GenTxid::Txid(parent_txid)};
-                    // Exclude m_lazy_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(nodeid, gtxid, current_time, /*p2p_inv=*/false);
+                auto add_orphan_reso_candidate = [&](const CTransactionRef& orphan_tx, const std::vector<Txid>& unique_parents, NodeId nodeid, std::chrono::microseconds now) {
+                    const auto& wtxid = orphan_tx->GetWitnessHash();
+                    if (auto delay{OrphanResolutionCandidate(nodeid, wtxid, unique_parents.size())}) {
+                        const auto& info = m_peer_info.at(nodeid).m_connection_info;
+                        m_orphanage.AddTx(orphan_tx, nodeid);
+
+                        // Treat finding orphan resolution candidate as equivalent to the peer announcing all missing parents
+                        // In the future, orphan resolution may include more explicit steps
+                        for (const auto& parent_txid : unique_parents) {
+                            m_txrequest.ReceivedInv(nodeid, GenTxid::Txid(parent_txid), info.m_preferred, now + *delay);
+                        }
+                        LogDebug(BCLog::TXPACKAGES, "added peer=%d as a candidate for resolving orphan %s\n", nodeid, wtxid.ToString());
                     }
-                }
+                };
 
-                // Potentially flip add_extra_compact_tx to false if AddTx returns false because the tx was already there
-                add_extra_compact_tx &= m_orphanage.AddTx(ptx, nodeid);
+                // If there is no candidate for orphan resolution, AddTx will not be called. This means
+                // that if a peer is overloading us with invs and orphans, they will eventually not be
+                // able to add any more transactions to the orphanage.
+                add_orphan_reso_candidate(ptx, unique_parents, nodeid, now);
+                for (const auto& candidate : m_txrequest.GetCandidatePeers(ptx)) {
+                    add_orphan_reso_candidate(ptx, unique_parents, candidate, now);
+                }
 
                 // Once added to the orphan pool, a tx is considered AlreadyHave, and we shouldn't request it anymore.
                 m_txrequest.ForgetTxHash(tx.GetHash());
                 m_txrequest.ForgetTxHash(tx.GetWitnessHash());
 
                 // DoS prevention: do not allow m_orphanage to grow unbounded (see CVE-2012-3789)
+                // Note that, if the orphanage reaches capacity, it's possible that we immediately evict
+                // the transaction we just added.
                 m_orphanage.LimitOrphans(m_opts.m_max_orphan_txs, m_opts.m_rng);
             } else {
                 unique_parents.clear();

src/node/txdownloadman_impl.h

@@ -163,7 +163,7 @@ public:
     /** Consider adding this tx hash to txrequest. Should be called whenever a new inv has been received.
      * Also called internally when a transaction is missing parents so that we can request them.
      */
-    bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv);
+    bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now);
 
     /** Get getdata requests to send. */
     std::vector<GenTxid> GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time);
@@ -189,6 +189,16 @@ public:
     void CheckIsEmpty(NodeId nodeid);
 
     std::vector<TxOrphanage::OrphanTxBase> GetOrphanTransactions() const;
+
+protected:
+    /** Helper for getting deduplicated vector of Txids in vin. */
+    std::vector<Txid> GetUniqueParents(const CTransaction& tx);
+
+    /** Determine candidacy (and delay) for potential orphan resolution candidate.
+     * @returns delay for orphan resolution if this peer is a good candidate for orphan resolution,
+     * std::nullopt if this peer cannot be added because it has reached download/orphanage limits.
+     * */
+    std::optional<std::chrono::seconds> OrphanResolutionCandidate(NodeId nodeid, const Wtxid& orphan_wtxid, size_t num_parents);
 };
 } // namespace node
 #endif // BITCOIN_NODE_TXDOWNLOADMAN_IMPL_H

src/rpc/mempool.cpp

@@ -845,7 +845,9 @@ static UniValue OrphanToJSON(const TxOrphanage::OrphanTxBase& orphan)
     o.pushKV("entry", int64_t{TicksSinceEpoch<std::chrono::seconds>(orphan.nTimeExpire - ORPHAN_TX_EXPIRE_TIME)});
     o.pushKV("expiration", int64_t{TicksSinceEpoch<std::chrono::seconds>(orphan.nTimeExpire)});
     UniValue from(UniValue::VARR);
-    from.push_back(orphan.fromPeer); // only one fromPeer for now
+    for (const auto fromPeer: orphan.announcers) {
+        from.push_back(fromPeer);
+    }
     o.pushKV("from", from);
     return o;
 }

src/test/fuzz/txdownloadman.cpp

@@ -228,7 +228,7 @@ FUZZ_TARGET(txdownloadman, .init = initialize)
                 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.AddTxAnnouncement(rand_peer, gtxid, time);
             },
             [&] {
                 txdownloadman.GetRequestsToSend(rand_peer, time);
@@ -372,7 +372,7 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize)
                 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());
+                txdownload_impl.AddTxAnnouncement(rand_peer, gtxid, time);
             },
             [&] {
                 const auto getdata_requests = txdownload_impl.GetRequestsToSend(rand_peer, time);

src/test/fuzz/txorphan.cpp

@@ -88,12 +88,6 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                     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
@@ -128,6 +122,18 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                         Assert(!have_tx || !add_tx);
                     }
                 },
+                [&] {
+                    bool have_tx = orphanage.HaveTx(tx->GetWitnessHash());
+                    bool have_tx_and_peer = orphanage.HaveTxFromPeer(tx->GetWitnessHash(), peer_id);
+                    // AddAnnouncer should return false if tx doesn't exist or we already HaveTxFromPeer.
+                    {
+                        bool added_announcer = orphanage.AddAnnouncer(tx->GetWitnessHash(), peer_id);
+                        // have_tx == false -> added_announcer == false
+                        Assert(have_tx || !added_announcer);
+                        // have_tx_and_peer == true -> added_announcer == false
+                        Assert(!have_tx_and_peer || !added_announcer);
+                    }
+                },
                 [&] {
                     bool have_tx = orphanage.HaveTx(tx->GetWitnessHash());
                     // EraseTx should return 0 if m_orphans doesn't have the tx
@@ -142,6 +148,7 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                 },
                 [&] {
                     orphanage.EraseForPeer(peer_id);
+                    Assert(!orphanage.HaveTxFromPeer(tx->GetWitnessHash(), peer_id));
                 },
                 [&] {
                     // test mocktime and expiry
@@ -157,5 +164,8 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
             ptx_potential_parent = tx;
         }
 
+        const bool have_tx{orphanage.HaveTx(tx->GetWitnessHash())};
+        const bool get_tx_nonnull{orphanage.GetTx(tx->GetWitnessHash()) != nullptr};
+        Assert(have_tx == get_tx_nonnull);
     }
 }

src/test/miner_tests.cpp

@@ -16,6 +16,7 @@
 #include <txmempool.h>
 #include <uint256.h>
 #include <util/check.h>
+#include <util/feefrac.h>
 #include <util/strencodings.h>
 #include <util/time.h>
 #include <util/translation.h>
@@ -25,6 +26,7 @@
 #include <test/util/setup_common.h>
 
 #include <memory>
+#include <vector>
 
 #include <boost/test/unit_test.hpp>
 
@@ -123,19 +125,22 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     tx.vout[0].nValue = 5000000000LL - 1000;
     // This tx has a low fee: 1000 satoshis
     Txid hashParentTx = tx.GetHash(); // save this txid for later use
-    AddToMempool(tx_mempool, entry.Fee(1000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
+    const auto parent_tx{entry.Fee(1000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx)};
+    AddToMempool(tx_mempool, parent_tx);
 
     // This tx has a medium fee: 10000 satoshis
     tx.vin[0].prevout.hash = txFirst[1]->GetHash();
     tx.vout[0].nValue = 5000000000LL - 10000;
     Txid hashMediumFeeTx = tx.GetHash();
-    AddToMempool(tx_mempool, entry.Fee(10000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
+    const auto medium_fee_tx{entry.Fee(10000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx)};
+    AddToMempool(tx_mempool, medium_fee_tx);
 
     // This tx has a high fee, but depends on the first transaction
     tx.vin[0].prevout.hash = hashParentTx;
     tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 50k satoshi fee
     Txid hashHighFeeTx = tx.GetHash();
-    AddToMempool(tx_mempool, entry.Fee(50000).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
+    const auto high_fee_tx{entry.Fee(50000).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx)};
+    AddToMempool(tx_mempool, high_fee_tx);
 
     std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options);
     BOOST_REQUIRE(block_template);
@@ -145,6 +150,21 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     BOOST_CHECK(block.vtx[2]->GetHash() == hashHighFeeTx);
     BOOST_CHECK(block.vtx[3]->GetHash() == hashMediumFeeTx);
 
+    // Test the inclusion of package feerates in the block template and ensure they are sequential.
+    const auto block_package_feerates = BlockAssembler{m_node.chainman->ActiveChainstate(), &tx_mempool, options}.CreateNewBlock()->m_package_feerates;
+    BOOST_CHECK(block_package_feerates.size() == 2);
+
+    // parent_tx and high_fee_tx are added to the block as a package.
+    const auto combined_txs_fee = parent_tx.GetFee() + high_fee_tx.GetFee();
+    const auto combined_txs_size = parent_tx.GetTxSize() + high_fee_tx.GetTxSize();
+    FeeFrac package_feefrac{combined_txs_fee, combined_txs_size};
+    // The package should be added first.
+    BOOST_CHECK(block_package_feerates[0] == package_feefrac);
+
+    // The medium_fee_tx should be added next.
+    FeeFrac medium_tx_feefrac{medium_fee_tx.GetFee(), medium_fee_tx.GetTxSize()};
+    BOOST_CHECK(block_package_feerates[1] == medium_tx_feefrac);
+
     // Test that a package below the block min tx fee doesn't get included
     tx.vin[0].prevout.hash = hashHighFeeTx;
     tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 0 fee

src/test/orphanage_tests.cpp

@@ -93,15 +93,6 @@ static bool EqualTxns(const std::set<CTransactionRef>& set_txns, const std::vect
     }
     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)
 {
@@ -132,7 +123,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         tx.vin[0].prevout.hash = Txid::FromUint256(m_rng.rand256());
         tx.vin[0].scriptSig << OP_1;
         tx.vout.resize(1);
-        tx.vout[0].nValue = 1*CENT;
+        tx.vout[0].nValue = i*CENT;
         tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
 
         orphanage.AddTx(MakeTransactionRef(tx), i);
@@ -148,7 +139,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         tx.vin[0].prevout.n = 0;
         tx.vin[0].prevout.hash = txPrev->GetHash();
         tx.vout.resize(1);
-        tx.vout[0].nValue = 1*CENT;
+        tx.vout[0].nValue = i*CENT;
         tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
         SignatureData empty;
         BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL, empty));
@@ -250,7 +241,9 @@ BOOST_AUTO_TEST_CASE(same_txid_diff_witness)
     // EraseTx fails as transaction by this wtxid doesn't exist.
     BOOST_CHECK_EQUAL(orphanage.EraseTx(mutated_wtxid), 0);
     BOOST_CHECK(orphanage.HaveTx(normal_wtxid));
+    BOOST_CHECK(orphanage.GetTx(normal_wtxid) == child_normal);
     BOOST_CHECK(!orphanage.HaveTx(mutated_wtxid));
+    BOOST_CHECK(orphanage.GetTx(mutated_wtxid) == nullptr);
 
     // Must succeed. Both transactions should be present in orphanage.
     BOOST_CHECK(orphanage.AddTx(child_mutated, peer));
@@ -310,9 +303,6 @@ BOOST_AUTO_TEST_CASE(get_children)
         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());
@@ -320,8 +310,6 @@ BOOST_AUTO_TEST_CASE(get_children)
         // 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
@@ -344,7 +332,6 @@ BOOST_AUTO_TEST_CASE(get_children)
             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:
@@ -352,7 +339,6 @@ BOOST_AUTO_TEST_CASE(get_children)
             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:
@@ -360,7 +346,6 @@ BOOST_AUTO_TEST_CASE(get_children)
             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:
@@ -368,7 +353,6 @@ BOOST_AUTO_TEST_CASE(get_children)
             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)));
         }
     }
 }
@@ -390,4 +374,184 @@ BOOST_AUTO_TEST_CASE(too_large_orphan_tx)
     BOOST_CHECK(orphanage.AddTx(MakeTransactionRef(tx), 0));
 }
 
+BOOST_AUTO_TEST_CASE(process_block)
+{
+    FastRandomContext det_rand{true};
+    TxOrphanageTest orphanage{det_rand};
+
+    // Create outpoints that will be spent by transactions in the block
+    std::vector<COutPoint> outpoints;
+    const uint32_t num_outpoints{6};
+    outpoints.reserve(num_outpoints);
+    for (uint32_t i{0}; i < num_outpoints; ++i) {
+        // All the hashes should be different, but change the n just in case.
+        outpoints.emplace_back(Txid::FromUint256(det_rand.rand256()), i);
+    }
+
+    CBlock block;
+    const NodeId node{0};
+
+    auto control_tx = MakeTransactionSpending({}, det_rand);
+    BOOST_CHECK(orphanage.AddTx(control_tx, node));
+
+    auto bo_tx_same_txid = MakeTransactionSpending({outpoints.at(0)}, det_rand);
+    BOOST_CHECK(orphanage.AddTx(bo_tx_same_txid, node));
+    block.vtx.emplace_back(bo_tx_same_txid);
+
+    // 2 transactions with the same txid but different witness
+    auto b_tx_same_txid_diff_witness = MakeTransactionSpending({outpoints.at(1)}, det_rand);
+    block.vtx.emplace_back(b_tx_same_txid_diff_witness);
+
+    auto o_tx_same_txid_diff_witness = MakeMutation(b_tx_same_txid_diff_witness);
+    BOOST_CHECK(orphanage.AddTx(o_tx_same_txid_diff_witness, node));
+
+    // 2 different transactions that spend the same input.
+    auto b_tx_conflict = MakeTransactionSpending({outpoints.at(2)}, det_rand);
+    block.vtx.emplace_back(b_tx_conflict);
+
+    auto o_tx_conflict = MakeTransactionSpending({outpoints.at(2)}, det_rand);
+    BOOST_CHECK(orphanage.AddTx(o_tx_conflict, node));
+
+    // 2 different transactions that have 1 overlapping input.
+    auto b_tx_conflict_partial = MakeTransactionSpending({outpoints.at(3), outpoints.at(4)}, det_rand);
+    block.vtx.emplace_back(b_tx_conflict_partial);
+
+    auto o_tx_conflict_partial_2 = MakeTransactionSpending({outpoints.at(4), outpoints.at(5)}, det_rand);
+    BOOST_CHECK(orphanage.AddTx(o_tx_conflict_partial_2, node));
+
+    orphanage.EraseForBlock(block);
+    for (const auto& expected_removed : {bo_tx_same_txid, o_tx_same_txid_diff_witness, o_tx_conflict, o_tx_conflict_partial_2}) {
+        const auto& expected_removed_wtxid = expected_removed->GetWitnessHash();
+        BOOST_CHECK(!orphanage.HaveTx(expected_removed_wtxid));
+    }
+    // Only remaining tx is control_tx
+    BOOST_CHECK_EQUAL(orphanage.Size(), 1);
+    BOOST_CHECK(orphanage.HaveTx(control_tx->GetWitnessHash()));
+}
+
+BOOST_AUTO_TEST_CASE(multiple_announcers)
+{
+    const NodeId node0{0};
+    const NodeId node1{1};
+    const NodeId node2{2};
+    size_t expected_total_count{0};
+    FastRandomContext det_rand{true};
+    TxOrphanageTest orphanage{det_rand};
+
+    // Check accounting per peer.
+    // Check that EraseForPeer works with multiple announcers.
+    {
+        auto ptx = MakeTransactionSpending({}, det_rand);
+        const auto& wtxid = ptx->GetWitnessHash();
+        BOOST_CHECK(orphanage.AddTx(ptx, node0));
+        BOOST_CHECK(orphanage.HaveTx(wtxid));
+        expected_total_count += 1;
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        // Adding again should do nothing.
+        BOOST_CHECK(!orphanage.AddTx(ptx, node0));
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        // We can add another tx with the same txid but different witness.
+        auto ptx_mutated{MakeMutation(ptx)};
+        BOOST_CHECK(orphanage.AddTx(ptx_mutated, node0));
+        BOOST_CHECK(orphanage.HaveTx(ptx_mutated->GetWitnessHash()));
+        expected_total_count += 1;
+
+        BOOST_CHECK(!orphanage.AddTx(ptx, node0));
+
+        // Adding a new announcer should not change overall accounting.
+        BOOST_CHECK(orphanage.AddAnnouncer(ptx->GetWitnessHash(), node2));
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        // If we already have this announcer, AddAnnouncer returns false.
+        BOOST_CHECK(orphanage.HaveTxFromPeer(ptx->GetWitnessHash(), node2));
+        BOOST_CHECK(!orphanage.AddAnnouncer(ptx->GetWitnessHash(), node2));
+
+        // Same with using AddTx for an existing tx, which is equivalent to using AddAnnouncer
+        BOOST_CHECK(!orphanage.AddTx(ptx, node1));
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        // if EraseForPeer is called for an orphan with multiple announcers, the orphanage should only
+        // erase that peer from the announcers set.
+        orphanage.EraseForPeer(node0);
+        BOOST_CHECK(orphanage.HaveTx(ptx->GetWitnessHash()));
+        BOOST_CHECK(!orphanage.HaveTxFromPeer(ptx->GetWitnessHash(), node0));
+        // node0 is the only one that announced ptx_mutated
+        BOOST_CHECK(!orphanage.HaveTx(ptx_mutated->GetWitnessHash()));
+        expected_total_count -= 1;
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        // EraseForPeer should delete the orphan if it's the only announcer left.
+        orphanage.EraseForPeer(node1);
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+        BOOST_CHECK(orphanage.HaveTx(ptx->GetWitnessHash()));
+        orphanage.EraseForPeer(node2);
+        expected_total_count -= 1;
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+        BOOST_CHECK(!orphanage.HaveTx(ptx->GetWitnessHash()));
+    }
+
+    // Check that erasure for blocks removes for all peers.
+    {
+        CBlock block;
+        auto tx_block = MakeTransactionSpending({}, det_rand);
+        block.vtx.emplace_back(tx_block);
+        BOOST_CHECK(orphanage.AddTx(tx_block, node0));
+        BOOST_CHECK(!orphanage.AddTx(tx_block, node1));
+
+        expected_total_count += 1;
+
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+
+        orphanage.EraseForBlock(block);
+
+        expected_total_count -= 1;
+
+        BOOST_CHECK_EQUAL(orphanage.Size(), expected_total_count);
+    }
+}
+BOOST_AUTO_TEST_CASE(peer_worksets)
+{
+    const NodeId node0{0};
+    const NodeId node1{1};
+    const NodeId node2{2};
+    FastRandomContext det_rand{true};
+    TxOrphanageTest orphanage{det_rand};
+    // AddChildrenToWorkSet should pick an announcer randomly
+    {
+        auto tx_missing_parent = MakeTransactionSpending({}, det_rand);
+        auto tx_orphan = MakeTransactionSpending({COutPoint{tx_missing_parent->GetHash(), 0}}, det_rand);
+        const auto& orphan_wtxid = tx_orphan->GetWitnessHash();
+
+        // All 3 peers are announcers.
+        BOOST_CHECK(orphanage.AddTx(tx_orphan, node0));
+        BOOST_CHECK(!orphanage.AddTx(tx_orphan, node1));
+        BOOST_CHECK(orphanage.AddAnnouncer(orphan_wtxid, node2));
+        for (NodeId node = node0; node <= node2; ++node) {
+            BOOST_CHECK(orphanage.HaveTxFromPeer(orphan_wtxid, node));
+        }
+
+        // Parent accepted: add child to all 3 worksets.
+        orphanage.AddChildrenToWorkSet(*tx_missing_parent);
+        BOOST_CHECK_EQUAL(orphanage.GetTxToReconsider(node0), tx_orphan);
+        BOOST_CHECK_EQUAL(orphanage.GetTxToReconsider(node1), tx_orphan);
+        // Don't call GetTxToReconsider(node2) yet because it mutates the workset.
+
+        // EraseForPeer also removes that tx from the workset.
+        orphanage.EraseForPeer(node0);
+        BOOST_CHECK_EQUAL(orphanage.GetTxToReconsider(node0), nullptr);
+
+        // However, the other peers' worksets are not touched.
+        BOOST_CHECK_EQUAL(orphanage.GetTxToReconsider(node2), tx_orphan);
+
+        // Delete this tx, clearing the orphanage.
+        BOOST_CHECK_EQUAL(orphanage.EraseTx(orphan_wtxid), 1);
+        BOOST_CHECK_EQUAL(orphanage.Size(), 0);
+        for (NodeId node = node0; node <= node2; ++node) {
+            BOOST_CHECK_EQUAL(orphanage.GetTxToReconsider(node), nullptr);
+            BOOST_CHECK(!orphanage.HaveTxFromPeer(orphan_wtxid, node));
+        }
+    }
+}
 BOOST_AUTO_TEST_SUITE_END()

src/test/txdownload_tests.cpp

@@ -146,8 +146,8 @@ BOOST_FIXTURE_TEST_CASE(tx_rejection_types, TestChain100Setup)
                     /*txid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_txid),
                     /*wtxid_recon=*/txdownload_impl.RecentRejectsReconsiderableFilter().contains(parent_wtxid),
                     /*keep=*/keep,
-                    /*txid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Txid(parent_txid), now, /*p2p_inv=*/true),
-                    /*wtxid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Wtxid(parent_wtxid), now, /*p2p_inv=*/true),
+                    /*txid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Txid(parent_txid), now),
+                    /*wtxid_inv=*/txdownload_impl.AddTxAnnouncement(nodeid, GenTxid::Wtxid(parent_wtxid), now),
                 };
                 BOOST_TEST_MESSAGE("Testing behavior for " << result << (segwit_parent ? " segwit " : " nonsegwit"));
                 actual_behavior.CheckEqual(expected_behavior, /*segwit=*/segwit_parent);
@@ -231,10 +231,14 @@ BOOST_FIXTURE_TEST_CASE(handle_missing_inputs, TestChain100Setup)
         // it's in RecentRejectsFilter. Specifically, the parent is allowed to be in
         // RecentRejectsReconsiderableFilter, but it cannot be in RecentRejectsFilter.
         const bool expect_keep_orphan = !parent_recent_rej;
+        const unsigned int expected_parents = parent_recent_rej || parent_recent_conf || parent_in_mempool ? 0 : 1;
+        // If we don't expect to keep the orphan then expected_parents is 0.
+        // !expect_keep_orphan => (expected_parents == 0)
+        BOOST_CHECK(expect_keep_orphan || expected_parents == 0);
         const auto ret_1p1c = txdownload_impl.MempoolRejectedTx(orphan, state_orphan, nodeid, /*first_time_failure=*/true);
         std::string err_msg;
         const bool ok = CheckOrphanBehavior(txdownload_impl, orphan, ret_1p1c, err_msg,
-                                            /*expect_orphan=*/expect_keep_orphan, /*expect_keep=*/true, /*expected_parents=*/expect_keep_orphan ? 1 : 0);
+                                            /*expect_orphan=*/expect_keep_orphan, /*expect_keep=*/true, /*expected_parents=*/expected_parents);
         BOOST_CHECK_MESSAGE(ok, err_msg);
     }
 
@@ -278,11 +282,12 @@ BOOST_FIXTURE_TEST_CASE(handle_missing_inputs, TestChain100Setup)
             for (int32_t i = 1; i < num_parents; ++i) {
                 txdownload_impl.RecentConfirmedTransactionsFilter().insert(parents[i]->GetHash().ToUint256());
             }
+            const unsigned int expected_parents = 1;
 
             const auto ret_1recon_conf = txdownload_impl.MempoolRejectedTx(orphan, state_orphan, nodeid, /*first_time_failure=*/true);
             std::string err_msg;
             const bool ok = CheckOrphanBehavior(txdownload_impl, orphan, ret_1recon_conf, err_msg,
-                                                /*expect_orphan=*/true, /*expect_keep=*/true, /*expected_parents=*/num_parents);
+                                                /*expect_orphan=*/true, /*expect_keep=*/true, /*expected_parents=*/expected_parents);
             BOOST_CHECK_MESSAGE(ok, err_msg);
         }
 

src/txorphanage.cpp

@@ -16,8 +16,11 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
 {
     const Txid& hash = tx->GetHash();
     const Wtxid& wtxid = tx->GetWitnessHash();
-    if (m_orphans.count(wtxid))
+    if (auto it{m_orphans.find(wtxid)}; it != m_orphans.end()) {
+        AddAnnouncer(wtxid, peer);
+        // No new orphan entry was created. An announcer may have been added.
         return false;
+    }
 
     // Ignore big transactions, to avoid a
     // send-big-orphans memory exhaustion attack. If a peer has a legitimate
@@ -33,7 +36,7 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
         return false;
     }
 
-    auto ret = m_orphans.emplace(wtxid, OrphanTx{{tx, peer, Now<NodeSeconds>() + ORPHAN_TX_EXPIRE_TIME}, m_orphan_list.size()});
+    auto ret = m_orphans.emplace(wtxid, OrphanTx{{tx, {peer}, Now<NodeSeconds>() + ORPHAN_TX_EXPIRE_TIME}, m_orphan_list.size()});
     assert(ret.second);
     m_orphan_list.push_back(ret.first);
     for (const CTxIn& txin : tx->vin) {
@@ -45,6 +48,20 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
     return true;
 }
 
+bool TxOrphanage::AddAnnouncer(const Wtxid& wtxid, NodeId peer)
+{
+    const auto it = m_orphans.find(wtxid);
+    if (it != m_orphans.end()) {
+        Assume(!it->second.announcers.empty());
+        const auto ret = it->second.announcers.insert(peer);
+        if (ret.second) {
+            LogDebug(BCLog::TXPACKAGES, "added peer=%d as announcer of orphan tx %s\n", peer, wtxid.ToString());
+            return true;
+        }
+    }
+    return false;
+}
+
 int TxOrphanage::EraseTx(const Wtxid& wtxid)
 {
     std::map<Wtxid, OrphanTx>::iterator it = m_orphans.find(wtxid);
@@ -89,9 +106,15 @@ void TxOrphanage::EraseForPeer(NodeId peer)
     while (iter != m_orphans.end())
     {
         // increment to avoid iterator becoming invalid after erasure
-        const auto& [wtxid, orphan] = *iter++;
-        if (orphan.fromPeer == peer) {
-            nErased += EraseTx(wtxid);
+        auto& [wtxid, orphan] = *iter++;
+        auto orphan_it = orphan.announcers.find(peer);
+        if (orphan_it != orphan.announcers.end()) {
+            orphan.announcers.erase(peer);
+
+            // No remaining annnouncers: clean up entry
+            if (orphan.announcers.empty()) {
+                nErased += EraseTx(orphan.tx->GetWitnessHash());
+            }
         }
     }
     if (nErased > 0) LogDebug(BCLog::TXPACKAGES, "Erased %d orphan transaction(s) from peer=%d\n", nErased, peer);
@@ -110,7 +133,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
         {
             std::map<Wtxid, OrphanTx>::iterator maybeErase = iter++;
             if (maybeErase->second.nTimeExpire <= nNow) {
-                nErased += EraseTx(maybeErase->second.tx->GetWitnessHash());
+                nErased += EraseTx(maybeErase->first);
             } else {
                 nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime);
             }
@@ -123,7 +146,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
     {
         // Evict a random orphan:
         size_t randompos = rng.randrange(m_orphan_list.size());
-        EraseTx(m_orphan_list[randompos]->second.tx->GetWitnessHash());
+        EraseTx(m_orphan_list[randompos]->first);
         ++nEvicted;
     }
     if (nEvicted > 0) LogDebug(BCLog::TXPACKAGES, "orphanage overflow, removed %u tx\n", nEvicted);
@@ -135,13 +158,17 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx)
         const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(tx.GetHash(), i));
         if (it_by_prev != m_outpoint_to_orphan_it.end()) {
             for (const auto& elem : it_by_prev->second) {
-                // Get this source peer's work set, emplacing an empty set if it didn't exist
-                // (note: if this peer wasn't still connected, we would have removed the orphan tx already)
-                std::set<Wtxid>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second;
-                // Add this tx to the work set
-                orphan_work_set.insert(elem->first);
-                LogDebug(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n",
-                         tx.GetHash().ToString(), tx.GetWitnessHash().ToString(), elem->second.fromPeer);
+                // Belt and suspenders, each orphan should always have at least 1 announcer.
+                if (!Assume(!elem->second.announcers.empty())) continue;
+                for (const auto announcer: elem->second.announcers) {
+                    // Get this source peer's work set, emplacing an empty set if it didn't exist
+                    // (note: if this peer wasn't still connected, we would have removed the orphan tx already)
+                    std::set<Wtxid>& orphan_work_set = m_peer_work_set.try_emplace(announcer).first->second;
+                    // Add this tx to the work set
+                    orphan_work_set.insert(elem->first);
+                    LogDebug(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n",
+                             tx.GetHash().ToString(), tx.GetWitnessHash().ToString(), announcer);
+                }
             }
         }
     }
@@ -152,6 +179,18 @@ bool TxOrphanage::HaveTx(const Wtxid& wtxid) const
     return m_orphans.count(wtxid);
 }
 
+CTransactionRef TxOrphanage::GetTx(const Wtxid& wtxid) const
+{
+    auto it = m_orphans.find(wtxid);
+    return it != m_orphans.end() ? it->second.tx : nullptr;
+}
+
+bool TxOrphanage::HaveTxFromPeer(const Wtxid& wtxid, NodeId peer) const
+{
+    auto it = m_orphans.find(wtxid);
+    return (it != m_orphans.end() && it->second.announcers.contains(peer));
+}
+
 CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer)
 {
     auto work_set_it = m_peer_work_set.find(peer);
@@ -219,7 +258,7 @@ std::vector<CTransactionRef> TxOrphanage::GetChildrenFromSamePeer(const CTransac
         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) {
+                if (elem->second.announcers.contains(nodeid)) {
                     iters.emplace_back(elem);
                 }
             }
@@ -248,42 +287,12 @@ std::vector<CTransactionRef> TxOrphanage::GetChildrenFromSamePeer(const CTransac
     return children_found;
 }
 
-std::vector<std::pair<CTransactionRef, NodeId>> TxOrphanage::GetChildrenFromDifferentPeer(const CTransactionRef& parent, NodeId nodeid) const
-{
-    // 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;
-}
-
 std::vector<TxOrphanage::OrphanTxBase> TxOrphanage::GetOrphanTransactions() const
 {
     std::vector<OrphanTxBase> ret;
     ret.reserve(m_orphans.size());
     for (auto const& o : m_orphans) {
-        ret.push_back({o.second.tx, o.second.fromPeer, o.second.nTimeExpire});
+        ret.push_back({o.second.tx, o.second.announcers, o.second.nTimeExpire});
     }
     return ret;
 }

src/txorphanage.h

@@ -30,9 +30,17 @@ public:
     /** Add a new orphan transaction */
     bool AddTx(const CTransactionRef& tx, NodeId peer);
 
+    /** Add an additional announcer to an orphan if it exists. Otherwise, do nothing. */
+    bool AddAnnouncer(const Wtxid& wtxid, NodeId peer);
+
+    CTransactionRef GetTx(const Wtxid& wtxid) const;
+
     /** Check if we already have an orphan transaction (by wtxid only) */
     bool HaveTx(const Wtxid& wtxid) const;
 
+    /** Check if a {tx, peer} exists in the orphanage.*/
+    bool HaveTxFromPeer(const Wtxid& wtxid, NodeId peer) const;
+
     /** Extract a transaction from a peer's work set
      *  Returns nullptr if there are no transactions to work on.
      *  Otherwise returns the transaction reference, and removes
@@ -43,7 +51,8 @@ public:
     /** Erase an orphan by wtxid */
     int EraseTx(const Wtxid& wtxid);
 
-    /** Erase all orphans announced by a peer (eg, after that peer disconnects) */
+    /** Maybe erase all orphans announced by a peer (eg, after that peer disconnects). If an orphan
+     * has been announced by another peer, don't erase, just remove this peer from the list of announcers. */
     void EraseForPeer(NodeId peer);
 
     /** Erase all orphans included in or invalidated by a new block */
@@ -62,10 +71,6 @@ public:
      * recent to least recent. */
     std::vector<CTransactionRef> GetChildrenFromSamePeer(const CTransactionRef& parent, NodeId nodeid) const;
 
-    /** 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;
-
     /** Return how many entries exist in the orphange */
     size_t Size() const
     {
@@ -75,7 +80,8 @@ public:
     /** Allows providing orphan information externally */
     struct OrphanTxBase {
         CTransactionRef tx;
-        NodeId fromPeer;
+        /** Peers added with AddTx or AddAnnouncer. */
+        std::set<NodeId> announcers;
         NodeSeconds nTimeExpire;
     };
 

src/txrequest.cpp

@@ -574,6 +574,23 @@ public:
         }
     }
 
+    std::vector<NodeId> GetCandidatePeers(const CTransactionRef& tx) const
+    {
+        // Search by txid and, if the tx has a witness, wtxid
+        std::vector<uint256> hashes{tx->GetHash().ToUint256()};
+        if (tx->HasWitness()) hashes.emplace_back(tx->GetWitnessHash().ToUint256());
+
+        std::vector<NodeId> result_peers;
+        for (const uint256& txhash : hashes) {
+            auto it = m_index.get<ByTxHash>().lower_bound(ByTxHashView{txhash, State::CANDIDATE_DELAYED, 0});
+            while (it != m_index.get<ByTxHash>().end() && it->m_txhash == txhash && it->GetState() != State::COMPLETED) {
+                result_peers.push_back(it->m_peer);
+                ++it;
+            }
+        }
+        return result_peers;
+    }
+
     void ReceivedInv(NodeId peer, const GenTxid& gtxid, bool preferred,
         std::chrono::microseconds reqtime)
     {
@@ -721,6 +738,7 @@ size_t TxRequestTracker::CountInFlight(NodeId peer) const { return m_impl->Count
 size_t TxRequestTracker::CountCandidates(NodeId peer) const { return m_impl->CountCandidates(peer); }
 size_t TxRequestTracker::Count(NodeId peer) const { return m_impl->Count(peer); }
 size_t TxRequestTracker::Size() const { return m_impl->Size(); }
+std::vector<NodeId> TxRequestTracker::GetCandidatePeers(const CTransactionRef& tx) const { return m_impl->GetCandidatePeers(tx); }
 void TxRequestTracker::SanityCheck() const { m_impl->SanityCheck(); }
 
 void TxRequestTracker::PostGetRequestableSanityCheck(std::chrono::microseconds now) const

src/txrequest.h

@@ -195,6 +195,9 @@ public:
     /** Count how many announcements are being tracked in total across all peers and transaction hashes. */
     size_t Size() const;
 
+    /** For some tx return all peers with non-COMPLETED announcements for its txid or wtxid. The resulting vector may contain duplicate NodeIds. */
+    std::vector<NodeId> GetCandidatePeers(const CTransactionRef& tx) const;
+
     /** Access to the internal priority computation (testing only) */
     uint64_t ComputePriority(const uint256& txhash, NodeId peer, bool preferred) const;
 

test/functional/p2p_1p1c_network.py

@@ -143,12 +143,6 @@ class PackageRelayTest(BitcoinTestFramework):
         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:

test/functional/p2p_opportunistic_1p1c.py

@@ -215,7 +215,7 @@ class PackageRelayTest(BitcoinTestFramework):
 
     @cleanup
     def test_orphan_consensus_failure(self):
-        self.log.info("Check opportunistic 1p1c logic with consensus-invalid orphan causes disconnect of the correct peer")
+        self.log.info("Check opportunistic 1p1c logic requires parent and child to be from the same peer")
         node = self.nodes[0]
         low_fee_parent = self.create_tx_below_mempoolminfee(self.wallet)
         coin = low_fee_parent["new_utxo"]
@@ -239,15 +239,17 @@ class PackageRelayTest(BitcoinTestFramework):
         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"]))
+        # 3. A different peer relays the parent. Package is not evaluated because the transactions
+        # were not sent from the same peer.
+        parent_sender.send_and_ping(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.
+        # 5. Have the other peer send the tx too, so that tx_orphan_bad_wit package is attempted.
+        bad_orphan_sender.send_message(msg_tx(low_fee_parent["tx"]))
         bad_orphan_sender.wait_for_disconnect()
 
         # The peer that didn't provide the orphan should not be disconnected.
@@ -279,20 +281,17 @@ class PackageRelayTest(BitcoinTestFramework):
         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))
+        # rejected for being too low feerate. It is not evaluated as a package because the child was
+        # sent from a different peer, so we don't find out that the child is consensus-invalid.
+        fake_parent_sender.send_and_ping(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.
+        # 5. 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)]))

test/functional/p2p_orphan_handling.py

@@ -58,6 +58,10 @@ def cleanup(func):
             self.generate(self.nodes[0], 1)
             self.nodes[0].disconnect_p2ps()
             self.nodes[0].bumpmocktime(LONG_TIME_SKIP)
+            # Check that mempool and orphanage have been cleared
+            assert_equal(0, len(self.nodes[0].getorphantxs()))
+            assert_equal(0, len(self.nodes[0].getrawmempool()))
+            self.wallet.rescan_utxos(include_mempool=True)
     return wrapper
 
 class PeerTxRelayer(P2PTxInvStore):
@@ -533,7 +537,7 @@ class OrphanHandlingTest(BitcoinTestFramework):
         assert tx_middle["txid"] in node_mempool
         assert tx_grandchild["txid"] in node_mempool
         assert_equal(node.getmempoolentry(tx_middle["txid"])["wtxid"], tx_middle["wtxid"])
-        assert_equal(len(node.getorphantxs()), 0)
+        self.wait_until(lambda: len(node.getorphantxs()) == 0)
 
     @cleanup
     def test_orphan_txid_inv(self):
@@ -585,7 +589,7 @@ class OrphanHandlingTest(BitcoinTestFramework):
         assert tx_parent["txid"] in node_mempool
         assert tx_child["txid"] in node_mempool
         assert_equal(node.getmempoolentry(tx_child["txid"])["wtxid"], tx_child["wtxid"])
-        assert_equal(len(node.getorphantxs()), 0)
+        self.wait_until(lambda: len(node.getorphantxs()) == 0)
 
     @cleanup
     def test_max_orphan_amount(self):
@@ -610,7 +614,7 @@ class OrphanHandlingTest(BitcoinTestFramework):
 
         peer_1.sync_with_ping()
         orphanage = node.getorphantxs()
-        assert_equal(len(orphanage), DEFAULT_MAX_ORPHAN_TRANSACTIONS)
+        self.wait_until(lambda: len(node.getorphantxs()) == DEFAULT_MAX_ORPHAN_TRANSACTIONS)
 
         for orphan in orphans:
             assert tx_in_orphanage(node, orphan)
@@ -626,8 +630,173 @@ class OrphanHandlingTest(BitcoinTestFramework):
         self.log.info("Clearing the orphanage")
         for index, parent_orphan in enumerate(parent_orphans):
             peer_1.send_and_ping(msg_tx(parent_orphan))
-        assert_equal(len(node.getorphantxs()),0)
+        self.wait_until(lambda: len(node.getorphantxs()) == 0)
 
+    @cleanup
+    def test_orphan_handling_prefer_outbound(self):
+        self.log.info("Test that the node prefers requesting from outbound peers")
+        node = self.nodes[0]
+        orphan_wtxid, orphan_tx, parent_tx = self.create_parent_and_child()
+        orphan_inv = CInv(t=MSG_WTX, h=int(orphan_wtxid, 16))
+
+        peer_inbound = node.add_p2p_connection(PeerTxRelayer())
+        peer_outbound = node.add_outbound_p2p_connection(PeerTxRelayer(), p2p_idx=1)
+
+        # Inbound peer relays the transaction.
+        peer_inbound.send_and_ping(msg_inv([orphan_inv]))
+        self.nodes[0].bumpmocktime(TXREQUEST_TIME_SKIP)
+        peer_inbound.wait_for_getdata([int(orphan_wtxid, 16)])
+
+        # Both peers send invs for the orphan, so the node can expect both to know its ancestors.
+        peer_outbound.send_and_ping(msg_inv([orphan_inv]))
+
+        peer_inbound.send_and_ping(msg_tx(orphan_tx))
+
+        # There should be 1 orphan with 2 announcers (we don't know what their peer IDs are)
+        orphanage = node.getorphantxs(verbosity=2)
+        assert_equal(orphanage[0]["wtxid"], orphan_wtxid)
+        assert_equal(len(orphanage[0]["from"]), 2)
+
+        # The outbound peer should be preferred for getting orphan parents
+        self.nodes[0].bumpmocktime(TXID_RELAY_DELAY)
+        peer_outbound.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+        # There should be no request to the inbound peer
+        peer_inbound.assert_never_requested(int(parent_tx.rehash(), 16))
+
+        self.log.info("Test that, if the preferred peer doesn't respond, the node sends another request")
+        self.nodes[0].bumpmocktime(GETDATA_TX_INTERVAL)
+        peer_inbound.sync_with_ping()
+        peer_inbound.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+    @cleanup
+    def test_announcers_before_and_after(self):
+        self.log.info("Test that the node uses all peers who announced the tx prior to realizing it's an orphan")
+        node = self.nodes[0]
+        orphan_wtxid, orphan_tx, parent_tx = self.create_parent_and_child()
+        orphan_inv = CInv(t=MSG_WTX, h=int(orphan_wtxid, 16))
+
+        # Announces before tx is sent, disconnects while node is requesting parents
+        peer_early_disconnected = node.add_outbound_p2p_connection(PeerTxRelayer(), p2p_idx=3)
+        # Announces before tx is sent, doesn't respond to parent request
+        peer_early_unresponsive = node.add_p2p_connection(PeerTxRelayer())
+
+        # Announces after tx is sent
+        peer_late_announcer = node.add_p2p_connection(PeerTxRelayer())
+
+        # Both peers send invs for the orphan, so the node can expect both to know its ancestors.
+        peer_early_disconnected.send_and_ping(msg_inv([orphan_inv]))
+        self.nodes[0].bumpmocktime(TXREQUEST_TIME_SKIP)
+        peer_early_disconnected.wait_for_getdata([int(orphan_wtxid, 16)])
+        peer_early_unresponsive.send_and_ping(msg_inv([orphan_inv]))
+        peer_early_disconnected.send_and_ping(msg_tx(orphan_tx))
+
+        # There should be 1 orphan with 2 announcers (we don't know what their peer IDs are)
+        orphanage = node.getorphantxs(verbosity=2)
+        assert_equal(len(orphanage), 1)
+        assert_equal(orphanage[0]["wtxid"], orphan_wtxid)
+        assert_equal(len(orphanage[0]["from"]), 2)
+
+        # Peer disconnects before responding to request
+        self.nodes[0].bumpmocktime(TXID_RELAY_DELAY)
+        peer_early_disconnected.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+        peer_early_disconnected.peer_disconnect()
+
+        # The orphan should have 1 announcer left after the node finishes disconnecting peer_early_disconnected.
+        self.wait_until(lambda: len(node.getorphantxs(verbosity=2)[0]["from"]) == 1)
+
+        # The node should retry with the other peer that announced the orphan earlier.
+        # This node's request was additionally delayed because it's an inbound peer.
+        self.nodes[0].bumpmocktime(NONPREF_PEER_TX_DELAY)
+        peer_early_unresponsive.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+        self.log.info("Test that the node uses peers who announce the tx after realizing it's an orphan")
+        peer_late_announcer.send_and_ping(msg_inv([orphan_inv]))
+
+        # The orphan should have 2 announcers now
+        orphanage = node.getorphantxs(verbosity=2)
+        assert_equal(orphanage[0]["wtxid"], orphan_wtxid)
+        assert_equal(len(orphanage[0]["from"]), 2)
+
+        self.nodes[0].bumpmocktime(GETDATA_TX_INTERVAL)
+        peer_late_announcer.wait_for_parent_requests([int(parent_tx.rehash(), 16)])
+
+    @cleanup
+    def test_parents_change(self):
+        self.log.info("Test that, if a parent goes missing during orphan reso, it is requested")
+        node = self.nodes[0]
+        # Orphan will have 2 parents, 1 missing and 1 already in mempool when received.
+        # Create missing parent.
+        parent_missing = self.wallet.create_self_transfer()
+
+        # Create parent that will already be in mempool, but become missing during orphan resolution.
+        # Get 3 UTXOs for replacement-cycled parent, UTXOS A, B, C
+        coin_A = self.wallet.get_utxo(confirmed_only=True)
+        coin_B = self.wallet.get_utxo(confirmed_only=True)
+        coin_C = self.wallet.get_utxo(confirmed_only=True)
+        # parent_peekaboo_AB spends A and B. It is replaced by tx_replacer_BC (conflicting UTXO B),
+        # and then replaced by tx_replacer_C (conflicting UTXO C). This replacement cycle is used to
+        # ensure that parent_peekaboo_AB can be reintroduced without requiring package RBF.
+        FEE_INCREMENT = 2400
+        parent_peekaboo_AB = self.wallet.create_self_transfer_multi(
+            utxos_to_spend=[coin_A, coin_B],
+            num_outputs=1,
+            fee_per_output=FEE_INCREMENT
+        )
+        tx_replacer_BC = self.wallet.create_self_transfer_multi(
+            utxos_to_spend=[coin_B, coin_C],
+            num_outputs=1,
+            fee_per_output=2*FEE_INCREMENT
+        )
+        tx_replacer_C = self.wallet.create_self_transfer(
+            utxo_to_spend=coin_C,
+            fee_per_output=3*FEE_INCREMENT
+        )
+
+        # parent_peekaboo_AB starts out in the mempool
+        node.sendrawtransaction(parent_peekaboo_AB["hex"])
+
+        orphan = self.wallet.create_self_transfer_multi(utxos_to_spend=[parent_peekaboo_AB["new_utxos"][0], parent_missing["new_utxo"]])
+        orphan_wtxid = orphan["wtxid"]
+        orphan_inv = CInv(t=MSG_WTX, h=int(orphan_wtxid, 16))
+
+        # peer1 sends the orphan and gets a request for the missing parent
+        peer1 = node.add_p2p_connection(PeerTxRelayer())
+        peer1.send_and_ping(msg_inv([orphan_inv]))
+        node.bumpmocktime(TXREQUEST_TIME_SKIP)
+        peer1.wait_for_getdata([int(orphan_wtxid, 16)])
+        peer1.send_and_ping(msg_tx(orphan["tx"]))
+        self.wait_until(lambda: node.getorphantxs(verbosity=0) == [orphan["txid"]])
+        node.bumpmocktime(NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY)
+        peer1.wait_for_getdata([int(parent_missing["txid"], 16)])
+
+        # Replace parent_peekaboo_AB so that is is a newly missing parent.
+        # Then, replace the replacement so that it can be resubmitted.
+        node.sendrawtransaction(tx_replacer_BC["hex"])
+        assert tx_replacer_BC["txid"] in node.getrawmempool()
+        node.sendrawtransaction(tx_replacer_C["hex"])
+        assert tx_replacer_BC["txid"] not in node.getrawmempool()
+        assert tx_replacer_C["txid"] in node.getrawmempool()
+
+        # Second peer is an additional announcer for this orphan
+        peer2 = node.add_p2p_connection(PeerTxRelayer())
+        peer2.send_and_ping(msg_inv([orphan_inv]))
+        assert_equal(len(node.getorphantxs(verbosity=2)[0]["from"]), 2)
+
+        # Disconnect peer1. peer2 should become the new candidate for orphan resolution.
+        peer1.peer_disconnect()
+        node.bumpmocktime(NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY)
+        self.wait_until(lambda: len(node.getorphantxs(verbosity=2)[0]["from"]) == 1)
+        # Both parents should be requested, now that they are both missing.
+        peer2.wait_for_parent_requests([int(parent_peekaboo_AB["txid"], 16), int(parent_missing["txid"], 16)])
+        peer2.send_and_ping(msg_tx(parent_missing["tx"]))
+        peer2.send_and_ping(msg_tx(parent_peekaboo_AB["tx"]))
+
+        final_mempool = node.getrawmempool()
+        assert parent_missing["txid"] in final_mempool
+        assert parent_peekaboo_AB["txid"] in final_mempool
+        assert orphan["txid"] in final_mempool
+        assert tx_replacer_C["txid"] in final_mempool
 
     def run_test(self):
         self.nodes[0].setmocktime(int(time.time()))
@@ -635,6 +804,7 @@ class OrphanHandlingTest(BitcoinTestFramework):
         self.generate(self.wallet_nonsegwit, 10)
         self.wallet = MiniWallet(self.nodes[0])
         self.generate(self.wallet, 160)
+
         self.test_arrival_timing_orphan()
         self.test_orphan_rejected_parents_exceptions()
         self.test_orphan_multiple_parents()
@@ -645,6 +815,9 @@ class OrphanHandlingTest(BitcoinTestFramework):
         self.test_same_txid_orphan_of_orphan()
         self.test_orphan_txid_inv()
         self.test_max_orphan_amount()
+        self.test_orphan_handling_prefer_outbound()
+        self.test_announcers_before_and_after()
+        self.test_parents_change()
 
 
 if __name__ == '__main__':

test/functional/p2p_segwit.py

@@ -2051,6 +2051,9 @@ class SegWitTest(BitcoinTestFramework):
             self.wtx_node.last_message.pop("getdata", None)
         test_transaction_acceptance(self.nodes[0], self.wtx_node, tx2, with_witness=True, accepted=False)
 
+        # Disconnect tx_node to avoid the possibility of it being selected for orphan resolution.
+        self.tx_node.peer_disconnect()
+
         # Expect a request for parent (tx) by txid despite use of WTX peer
         self.wtx_node.wait_for_getdata([tx.sha256], timeout=60)
         with p2p_lock:

test/functional/rpc_orphans.py

@@ -10,7 +10,12 @@ from test_framework.mempool_util import (
     ORPHAN_TX_EXPIRE_TIME,
     tx_in_orphanage,
 )
-from test_framework.messages import msg_tx
+from test_framework.messages import (
+    CInv,
+    msg_inv,
+    msg_tx,
+    MSG_WTX,
+)
 from test_framework.p2p import P2PInterface
 from test_framework.util import (
     assert_equal,
@@ -106,13 +111,19 @@ class OrphanRPCsTest(BitcoinTestFramework):
 
         self.log.info("Check that orphan 1 and 2 were from different peers")
         assert orphanage[0]["from"][0] != orphanage[1]["from"][0]
+        peer_ids = [orphanage[0]["from"][0], orphanage[1]["from"][0]]
 
         self.log.info("Unorphan child 2")
         peer_2.send_and_ping(msg_tx(tx_parent_2["tx"]))
         assert not tx_in_orphanage(node, tx_child_2["tx"])
 
+        self.log.info("Check that additional announcers are reflected in RPC result")
+        peer_2.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=int(tx_child_1["wtxid"], 16))]))
+
+        orphanage = node.getorphantxs(verbosity=2)
+        assert_equal(set(orphanage[0]["from"]), set(peer_ids))
+
         self.log.info("Checking orphan details")
-        orphanage = node.getorphantxs(verbosity=1)
         assert_equal(len(node.getorphantxs()), 1)
         orphan_1 = orphanage[0]
         self.orphan_details_match(orphan_1, tx_child_1, verbosity=1)