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

Code before:

std::vector<node::TxOrphanage::OrphanInfo> PeerManagerImpl::GetOrphanTransactions()
{
    LOCK(m_tx_download_mutex);
    return m_txdownloadman.GetOrphanTransactions();
}

PeerManagerInfo PeerManagerImpl::GetInfo() const
{
    return PeerManagerInfo{
        .median_outbound_time_offset = m_outbound_time_offsets.Median(),
        .ignores_incoming_txs = m_opts.ignore_incoming_txs,
    };
}

std::vector<PrivateBroadcast::TxBroadcastInfo> PeerManagerImpl::GetPrivateBroadcastInfo() const
{
    return m_tx_for_private_broadcast.GetBroadcastInfo();
}

std::vector<CTransactionRef> PeerManagerImpl::AbortPrivateBroadcast(const uint256& id)
{
    const auto snapshot{m_tx_for_private_broadcast.GetBroadcastInfo()};
    std::vector<CTransactionRef> to_remove;
    to_remove.reserve(snapshot.size());
    for (const auto& entry : snapshot) {
        if (entry.tx->GetHash().ToUint256() == id || entry.tx->GetWitnessHash().ToUint256() == id) {
            to_remove.push_back(entry.tx);
        }
    }

    size_t connections_cancelled{0};
    for (const auto& tx : to_remove) {
        const auto peer_acks{m_tx_for_private_broadcast.Remove(tx)};
        if (!peer_acks.has_value()) continue;
        if (NUM_PRIVATE_BROADCAST_PER_TX > peer_acks.value()) {
            connections_cancelled += (NUM_PRIVATE_BROADCAST_PER_TX - peer_acks.value());
        }
    }

    if (connections_cancelled > 0) {
        m_connman.m_private_broadcast.NumToOpenSub(connections_cancelled);
    }

    return to_remove;
}

void PeerManagerImpl::AddToCompactExtraTransactions(const CTransactionRef& tx)
{
    if (m_opts.max_extra_txs <= 0)
        return;

Review comment (suggestion):
I think there might be a small race here : we take a snapshot and then remove in a separate pass, but we don't hold one lock across both operations.

If another thread removes a tx between `GetBroadcastInfo()` and `Remove()`, this method can currently return a tx that this call did not actually remove.
Given the contract (`Transactions removed from the private broadcast queue`), should we return only txs removed by this call (or otherwise relax the docs)?

Maybe something like:
```C++
std::vector<CTransactionRef> PeerManagerImpl::AbortPrivateBroadcast(const uint256& id)
{
    const auto snapshot{m_tx_for_private_broadcast.GetBroadcastInfo()};
    std::vector<CTransactionRef> removed_txs;
    removed_txs.reserve(snapshot.size());

    size_t connections_cancelled{0};
    for (const auto& [tx, _] : snapshot) {
        if (tx->GetHash().ToUint256() != id && tx->GetWitnessHash().ToUint256() != id) continue;
        if (const auto peer_acks{m_tx_for_private_broadcast.Remove(tx)}) {
            removed_txs.push_back(tx);
            if (NUM_PRIVATE_BROADCAST_PER_TX > *peer_acks) {
                connections_cancelled += (NUM_PRIVATE_BROADCAST_PER_TX - *peer_acks);
            }
        }
    }
    m_connman.m_private_broadcast.NumToOpenSub(connections_cancelled);

    return removed_txs;
}
```

Code after:

std::vector<node::TxOrphanage::OrphanInfo> PeerManagerImpl::GetOrphanTransactions()
{
    LOCK(m_tx_download_mutex);
    return m_txdownloadman.GetOrphanTransactions();
}

PeerManagerInfo PeerManagerImpl::GetInfo() const
{
    return PeerManagerInfo{
        .median_outbound_time_offset = m_outbound_time_offsets.Median(),
        .ignores_incoming_txs = m_opts.ignore_incoming_txs,
    };
}

std::vector<PrivateBroadcast::TxBroadcastInfo> PeerManagerImpl::GetPrivateBroadcastInfo() const
{
    return m_tx_for_private_broadcast.GetBroadcastInfo();
}

std::vector<CTransactionRef> PeerManagerImpl::AbortPrivateBroadcast(const uint256& id)
{
    const auto snapshot{m_tx_for_private_broadcast.GetBroadcastInfo()};
    std::vector<CTransactionRef> removed_txs;

    size_t connections_cancelled{0};
    for (const auto& [tx, _] : snapshot) {
        if (tx->GetHash().ToUint256() != id && tx->GetWitnessHash().ToUint256() != id) continue;
        if (const auto peer_acks{m_tx_for_private_broadcast.Remove(tx)}) {
            removed_txs.push_back(tx);
            if (NUM_PRIVATE_BROADCAST_PER_TX > *peer_acks) {
                connections_cancelled += (NUM_PRIVATE_BROADCAST_PER_TX - *peer_acks);
            }
        }
    }
    m_connman.m_private_broadcast.NumToOpenSub(connections_cancelled);

    return removed_txs;
}

void PeerManagerImpl::AddToCompactExtraTransactions(const CTransactionRef& tx)
{
    if (m_opts.max_extra_txs <= 0)
        return;
    if (!vExtraTxnForCompact.size())
        vExtraTxnForCompact.resize(m_opts.max_extra_txs);
    vExtraTxnForCompact[vExtraTxnForCompactIt] = std::make_pair(tx->GetWitnessHash(), tx);
    vExtraTxnForCompactIt = (vExtraTxnForCompactIt + 1) % m_opts.max_extra_txs;
}

void PeerManagerImpl::Misbehaving(Peer& peer, const std::string& message)