Repository: bitcoin/bitcoin
File: src/node/txdownloadman_impl.cpp (C++)

Code before:
                } else if (RecentRejectsReconsiderableFilter().contains(parent_txid) &&
                           !m_opts.m_mempool.exists(GenTxid::Txid(parent_txid))) {
                    // More than 1 parent in m_lazy_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) {
                // 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);

                auto add_orphan_reso_candidate = [&](const CTransactionRef& orphan_tx, const std::vector<Txid>& unique_parents, NodeId nodeid, std::chrono::microseconds now) {
                    if (AddOrphanParentAnnouncements(unique_parents, orphan_tx->GetWitnessHash(), nodeid, now)) {
                        m_orphanage.AddTx(orphan_tx, 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();
                LogDebug(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s (wtxid=%s)\n",
                         tx.GetHash().ToString(),
                         tx.GetWitnessHash().ToString());

Review comment (suggestion):
I find the flow of logic a bit convoluted:

The check whether a peer is a orphan resolution candidate happens in `OrphanResolutionCandidate`, which also calculates the delay and is called from `AddOrphanParentAnnouncements`.

I would find the folllowing kind of of flow easier to understand (no need for `std::optional`, functions do exactly what their name suggests they do):
```
if (IsOrphanCandidate()) { // only checks if peer is a candidate
   AddOrphanParentAnnouncements(); // delay is calculated here or in a submethod
   m_orphanage.AddTx() / m_orphanage.AddAnnouncer();
}
```


Code after:
                    // 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) {
                // 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);

                // 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.
                //
                // Search by txid and, if the tx has a witness, wtxid
                std::vector<NodeId> orphan_resolution_candidates{nodeid};
                m_txrequest.GetCandidatePeers(ptx->GetHash().ToUint256(), orphan_resolution_candidates);
                if (ptx->HasWitness()) m_txrequest.GetCandidatePeers(ptx->GetWitnessHash().ToUint256(), orphan_resolution_candidates);

                for (const auto& nodeid : orphan_resolution_candidates) {
                    if (MaybeAddOrphanResolutionCandidate(unique_parents, ptx->GetWitnessHash(), nodeid, now)) {
                        m_orphanage.AddTx(ptx, nodeid);
                    }
                }

                // 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();
                LogDebug(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s (wtxid=%s)\n",
                         tx.GetHash().ToString(),
                         tx.GetWitnessHash().ToString());
                // We will continue to reject this tx since it has rejected
                // parents so avoid re-requesting it from other peers.