Merge bitcoin/bitcoin#26569: p2p: Ensure transaction announcements are only queued for fully connected peers

8f2dac5409 [test] Add p2p_tx_privacy.py (dergoegge)
ce63fca13e [net processing] Assume that TxRelay::m_tx_inventory_to_send is empty pre-verack (dergoegge)
845e3a34c4 [net processing] Ensure transaction announcements are only queued for fully connected peers (dergoegge)

Pull request description:

  `TxRelay::m_next_inv_send_time` is initialized to 0, which means that any txids in `TxRelay::m_tx_inventory_to_send` will be announced on the first call to `PeerManagerImpl::SendMessages` for a fully connected peer (i.e. it completed the version handshake).

  Prior to #21160, `TxRelay::m_tx_inventory_to_send` was guaranteed to be empty on the first `SendMessages` call, as transaction announcements were only queued for fully connected peers. #21160 replaced a `CConnman::ForEachNode` call with a loop over `PeerManagerImpl::m_peer_map`, in which the txid for a transaction to be relayed is added to `TxRelay::m_tx_inventory_to_send` for all peers. Even for those peers that have not completed the version handshake. Prior to the PR this was not the case as `ForEachNode` has a "fully connected check" before calling a function for each node.

ACKs for top commit:
  MarcoFalke:
    ACK 8f2dac5409 🔝
  jnewbery:
    utACK 8f2dac5409

Tree-SHA512: e9eaccf7e00633ee0806fff1068b0e413a69a5e389d96c9659f68079915a6381ad5040c61f716cfcde77931d1b563b1049da97a232a95c6cd8355bd3d13404b9
This commit is contained in:
fanquake
2022-12-02 14:51:24 +00:00
3 changed files with 102 additions and 2 deletions

View File

@@ -295,7 +295,7 @@ struct Peer {
std::atomic<std::chrono::seconds> m_last_mempool_req{0s};
/** The next time after which we will send an `inv` message containing
* transaction announcements to this peer. */
std::chrono::microseconds m_next_inv_send_time GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0};
std::chrono::microseconds m_next_inv_send_time GUARDED_BY(m_tx_inventory_mutex){0};
/** Minimum fee rate with which to filter transaction announcements to this node. See BIP133. */
std::atomic<CAmount> m_fee_filter_received{0};
@@ -2017,8 +2017,15 @@ void PeerManagerImpl::RelayTransaction(const uint256& txid, const uint256& wtxid
auto tx_relay = peer.GetTxRelay();
if (!tx_relay) continue;
const uint256& hash{peer.m_wtxid_relay ? wtxid : txid};
LOCK(tx_relay->m_tx_inventory_mutex);
// Only queue transactions for announcement once the version handshake
// is completed. The time of arrival for these transactions is
// otherwise at risk of leaking to a spy, if the spy is able to
// distinguish transactions received during the handshake from the rest
// in the announcement.
if (tx_relay->m_next_inv_send_time == 0s) continue;
const uint256& hash{peer.m_wtxid_relay ? wtxid : txid};
if (!tx_relay->m_tx_inventory_known_filter.contains(hash)) {
tx_relay->m_tx_inventory_to_send.insert(hash);
}
@@ -3424,6 +3431,20 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
}
}
if (auto tx_relay = peer->GetTxRelay()) {
// `TxRelay::m_tx_inventory_to_send` must be empty before the
// version handshake is completed as
// `TxRelay::m_next_inv_send_time` is first initialised in
// `SendMessages` after the verack is received. Any transactions
// received during the version handshake would otherwise
// immediately be advertised without random delay, potentially
// leaking the time of arrival to a spy.
Assume(WITH_LOCK(
tx_relay->m_tx_inventory_mutex,
return tx_relay->m_tx_inventory_to_send.empty() &&
tx_relay->m_next_inv_send_time == 0s));
}
pfrom.fSuccessfullyConnected = true;
return;
}