mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-01-21 07:39:08 +01:00
Merge bitcoin/bitcoin#27675: p2p: Drop m_recently_announced_invs bloom filter
fb02ba3c5fmempool_entry: improve struct packing (Anthony Towns)1a118062fbnet_processing: Clean up INVENTORY_BROADCAST_MAX constants (Anthony Towns)6fa49937e4test: Check tx from disconnected block is immediately requestable (glozow)e4ffabbffanet_processing: don't add txids to m_tx_inventory_known_filter (Anthony Towns)6ec1809d33net_processing: drop m_recently_announced_invs bloom filter (Anthony Towns)a70beafdb2validation: when adding txs due to a block reorg, allow immediate relay (Anthony Towns)1e9684f39fmempool_entry: add mempool entry sequence number (Anthony Towns) Pull request description: This PR replaces the `m_recently_announced_invs` bloom filter with a simple sequence number tracking the mempool state when we last considered sending an INV message to a node. This saves 33kB per peer (or more if we raise the rate at which we relay transactions over the network, in which case we would need to increase the size of the bloom filter proportionally). The philosophy here (compare with #18861 and #19109) is that we consider the rate limiting on INV messages to only be about saving bandwidth and not protecting privacy, and therefore after you receive an INV message, it's immediately fair game to request any transaction that was in the mempool at the time the INV message was sent. We likewise consider the BIP 133 feefilter and BIP 37 bloom filters to be bandwidth optimisations here, and treat transactions as requestable if they would have been announced without those filters. Given that philosophy, tracking the timestamp of the last INV message and comparing that against the mempool entry time allows removal of each of `m_recently_announced_invs`, `m_last_mempool_req` and `UNCONDITIONAL_RELAY_DELAY` and associated logic. ACKs for top commit: naumenkogs: ACKfb02ba3c5famitiuttarwar: review ACKfb02ba3c5fglozow: reACKfb02ba3c5fTree-SHA512: cbba5ee04c86df26b6057f3654c00a2b45ec94d354f4f157a769cecdaa0b509edaac02b3128afba39b023e82473fc5e28c915a787f84457ffe66638c6ac9c2d4
This commit is contained in:
@@ -51,8 +51,6 @@
|
||||
#include <optional>
|
||||
#include <typeinfo>
|
||||
|
||||
/** How long a transaction has to be in the mempool before it can unconditionally be relayed. */
|
||||
static constexpr auto UNCONDITIONAL_RELAY_DELAY = 2min;
|
||||
/** Headers download timeout.
|
||||
* Timeout = base + per_header * (expected number of headers) */
|
||||
static constexpr auto HEADERS_DOWNLOAD_TIMEOUT_BASE = 15min;
|
||||
@@ -149,15 +147,12 @@ static constexpr auto OUTBOUND_INVENTORY_BROADCAST_INTERVAL{2s};
|
||||
/** Maximum rate of inventory items to send per second.
|
||||
* Limits the impact of low-fee transaction floods. */
|
||||
static constexpr unsigned int INVENTORY_BROADCAST_PER_SECOND = 7;
|
||||
/** Target number of tx inventory items to send per transmission. */
|
||||
static constexpr unsigned int INVENTORY_BROADCAST_TARGET = INVENTORY_BROADCAST_PER_SECOND * count_seconds(INBOUND_INVENTORY_BROADCAST_INTERVAL);
|
||||
/** Maximum number of inventory items to send per transmission. */
|
||||
static constexpr unsigned int INVENTORY_BROADCAST_MAX = INVENTORY_BROADCAST_PER_SECOND * count_seconds(INBOUND_INVENTORY_BROADCAST_INTERVAL);
|
||||
/** The number of most recently announced transactions a peer can request. */
|
||||
static constexpr unsigned int INVENTORY_MAX_RECENT_RELAY = 3500;
|
||||
/** Verify that INVENTORY_MAX_RECENT_RELAY is enough to cache everything typically
|
||||
* relayed before unconditional relay from the mempool kicks in. This is only a
|
||||
* lower bound, and it should be larger to account for higher inv rate to outbound
|
||||
* peers, and random variations in the broadcast mechanism. */
|
||||
static_assert(INVENTORY_MAX_RECENT_RELAY >= INVENTORY_BROADCAST_PER_SECOND * UNCONDITIONAL_RELAY_DELAY / std::chrono::seconds{1}, "INVENTORY_RELAY_MAX too low");
|
||||
static constexpr unsigned int INVENTORY_BROADCAST_MAX = 1000;
|
||||
static_assert(INVENTORY_BROADCAST_MAX >= INVENTORY_BROADCAST_TARGET, "INVENTORY_BROADCAST_MAX too low");
|
||||
static_assert(INVENTORY_BROADCAST_MAX <= MAX_PEER_TX_ANNOUNCEMENTS, "INVENTORY_BROADCAST_MAX too high");
|
||||
/** Average delay between feefilter broadcasts in seconds. */
|
||||
static constexpr auto AVG_FEEFILTER_BROADCAST_INTERVAL{10min};
|
||||
/** Maximum feefilter broadcast delay after significant change. */
|
||||
@@ -273,13 +268,10 @@ struct Peer {
|
||||
/** A bloom filter for which transactions to announce to the peer. See BIP37. */
|
||||
std::unique_ptr<CBloomFilter> m_bloom_filter PT_GUARDED_BY(m_bloom_filter_mutex) GUARDED_BY(m_bloom_filter_mutex){nullptr};
|
||||
|
||||
/** A rolling bloom filter of all announced tx CInvs to this peer */
|
||||
CRollingBloomFilter m_recently_announced_invs GUARDED_BY(NetEventsInterface::g_msgproc_mutex){INVENTORY_MAX_RECENT_RELAY, 0.000001};
|
||||
|
||||
mutable RecursiveMutex m_tx_inventory_mutex;
|
||||
/** A filter of all the txids and wtxids that the peer has announced to
|
||||
/** A filter of all the (w)txids that the peer has announced to
|
||||
* us or we have announced to the peer. We use this to avoid announcing
|
||||
* the same txid/wtxid to a peer that already has the transaction. */
|
||||
* the same (w)txid to a peer that already has the transaction. */
|
||||
CRollingBloomFilter m_tx_inventory_known_filter GUARDED_BY(m_tx_inventory_mutex){50000, 0.000001};
|
||||
/** Set of transaction ids we still have to announce (txid for
|
||||
* non-wtxid-relay peers, wtxid for wtxid-relay peers). We use the
|
||||
@@ -290,11 +282,12 @@ struct Peer {
|
||||
* permitted if the peer has NetPermissionFlags::Mempool or we advertise
|
||||
* NODE_BLOOM. See BIP35. */
|
||||
bool m_send_mempool GUARDED_BY(m_tx_inventory_mutex){false};
|
||||
/** The last time a BIP35 `mempool` request was serviced. */
|
||||
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(m_tx_inventory_mutex){0};
|
||||
/** The mempool sequence num at which we sent the last `inv` message to this peer.
|
||||
* Can relay txs with lower sequence numbers than this (see CTxMempool::info_for_relay). */
|
||||
uint64_t m_last_inv_sequence GUARDED_BY(NetEventsInterface::g_msgproc_mutex){1};
|
||||
|
||||
/** Minimum fee rate with which to filter transaction announcements to this node. See BIP133. */
|
||||
std::atomic<CAmount> m_fee_filter_received{0};
|
||||
@@ -907,7 +900,7 @@ private:
|
||||
std::atomic<std::chrono::seconds> m_last_tip_update{0s};
|
||||
|
||||
/** Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). */
|
||||
CTransactionRef FindTxForGetData(const Peer::TxRelay& tx_relay, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now)
|
||||
CTransactionRef FindTxForGetData(const Peer::TxRelay& tx_relay, const GenTxid& gtxid)
|
||||
EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, NetEventsInterface::g_msgproc_mutex);
|
||||
|
||||
void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic<bool>& interruptMsgProc)
|
||||
@@ -2288,22 +2281,14 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv&
|
||||
}
|
||||
}
|
||||
|
||||
CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer::TxRelay& tx_relay, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now)
|
||||
CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer::TxRelay& tx_relay, const GenTxid& gtxid)
|
||||
{
|
||||
auto txinfo = m_mempool.info(gtxid);
|
||||
// If a tx was in the mempool prior to the last INV for this peer, permit the request.
|
||||
auto txinfo = m_mempool.info_for_relay(gtxid, tx_relay.m_last_inv_sequence);
|
||||
if (txinfo.tx) {
|
||||
// If a TX could have been INVed in reply to a MEMPOOL request,
|
||||
// or is older than UNCONDITIONAL_RELAY_DELAY, permit the request
|
||||
// unconditionally.
|
||||
if ((mempool_req.count() && txinfo.m_time <= mempool_req) || txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY) {
|
||||
return std::move(txinfo.tx);
|
||||
}
|
||||
return std::move(txinfo.tx);
|
||||
}
|
||||
|
||||
// Otherwise, the transaction might have been announced recently.
|
||||
bool recent = tx_relay.m_recently_announced_invs.contains(gtxid.GetHash());
|
||||
if (recent && txinfo.tx) return std::move(txinfo.tx);
|
||||
|
||||
// Or it might be from the most recent block
|
||||
{
|
||||
LOCK(m_most_recent_block_mutex);
|
||||
@@ -2326,10 +2311,6 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic
|
||||
std::vector<CInv> vNotFound;
|
||||
const CNetMsgMaker msgMaker(pfrom.GetCommonVersion());
|
||||
|
||||
const auto now{GetTime<std::chrono::seconds>()};
|
||||
// Get last mempool request time
|
||||
const auto mempool_req = tx_relay != nullptr ? tx_relay->m_last_mempool_req.load() : std::chrono::seconds::min();
|
||||
|
||||
// Process as many TX items from the front of the getdata queue as
|
||||
// possible, since they're common and it's efficient to batch process
|
||||
// them.
|
||||
@@ -2347,33 +2328,12 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic
|
||||
continue;
|
||||
}
|
||||
|
||||
CTransactionRef tx = FindTxForGetData(*tx_relay, ToGenTxid(inv), mempool_req, now);
|
||||
CTransactionRef tx = FindTxForGetData(*tx_relay, ToGenTxid(inv));
|
||||
if (tx) {
|
||||
// WTX and WITNESS_TX imply we serialize with witness
|
||||
int nSendFlags = (inv.IsMsgTx() ? SERIALIZE_TRANSACTION_NO_WITNESS : 0);
|
||||
m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *tx));
|
||||
m_mempool.RemoveUnbroadcastTx(tx->GetHash());
|
||||
// As we're going to send tx, make sure its unconfirmed parents are made requestable.
|
||||
std::vector<uint256> parent_ids_to_add;
|
||||
{
|
||||
LOCK(m_mempool.cs);
|
||||
auto tx_iter = m_mempool.GetIter(tx->GetHash());
|
||||
if (tx_iter) {
|
||||
const CTxMemPoolEntry::Parents& parents = (*tx_iter)->GetMemPoolParentsConst();
|
||||
parent_ids_to_add.reserve(parents.size());
|
||||
for (const CTxMemPoolEntry& parent : parents) {
|
||||
if (parent.GetTime() > now - UNCONDITIONAL_RELAY_DELAY) {
|
||||
parent_ids_to_add.push_back(parent.GetTx().GetHash());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
for (const uint256& parent_txid : parent_ids_to_add) {
|
||||
// Relaying a transaction with a recent but unconfirmed parent.
|
||||
if (WITH_LOCK(tx_relay->m_tx_inventory_mutex, return !tx_relay->m_tx_inventory_known_filter.contains(parent_txid))) {
|
||||
tx_relay->m_recently_announced_invs.insert(parent_txid);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
vNotFound.push_back(inv);
|
||||
}
|
||||
@@ -4131,14 +4091,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
|
||||
|
||||
const uint256& hash = peer->m_wtxid_relay ? wtxid : txid;
|
||||
AddKnownTx(*peer, hash);
|
||||
if (peer->m_wtxid_relay && txid != wtxid) {
|
||||
// Insert txid into m_tx_inventory_known_filter, even for
|
||||
// wtxidrelay peers. This prevents re-adding of
|
||||
// unconfirmed parents to the recently_announced
|
||||
// filter, when a child tx is requested. See
|
||||
// ProcessGetData().
|
||||
AddKnownTx(*peer, txid);
|
||||
}
|
||||
|
||||
LOCK(cs_main);
|
||||
|
||||
@@ -5684,7 +5636,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
|
||||
std::vector<CInv> vInv;
|
||||
{
|
||||
LOCK(peer->m_block_inv_mutex);
|
||||
vInv.reserve(std::max<size_t>(peer->m_blocks_for_inv_relay.size(), INVENTORY_BROADCAST_MAX));
|
||||
vInv.reserve(std::max<size_t>(peer->m_blocks_for_inv_relay.size(), INVENTORY_BROADCAST_TARGET));
|
||||
|
||||
// Add blocks
|
||||
for (const uint256& hash : peer->m_blocks_for_inv_relay) {
|
||||
@@ -5736,14 +5688,12 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
|
||||
if (!tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue;
|
||||
}
|
||||
tx_relay->m_tx_inventory_known_filter.insert(hash);
|
||||
// Responses to MEMPOOL requests bypass the m_recently_announced_invs filter.
|
||||
vInv.push_back(inv);
|
||||
if (vInv.size() == MAX_INV_SZ) {
|
||||
m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv));
|
||||
vInv.clear();
|
||||
}
|
||||
}
|
||||
tx_relay->m_last_mempool_req = std::chrono::duration_cast<std::chrono::seconds>(current_time);
|
||||
}
|
||||
|
||||
// Determine transactions to relay
|
||||
@@ -5763,8 +5713,8 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
|
||||
// especially since we have many peers and some will draw much shorter delays.
|
||||
unsigned int nRelayedTransactions = 0;
|
||||
LOCK(tx_relay->m_bloom_filter_mutex);
|
||||
size_t broadcast_max{INVENTORY_BROADCAST_MAX + (tx_relay->m_tx_inventory_to_send.size()/1000)*5};
|
||||
broadcast_max = std::min<size_t>(1000, broadcast_max);
|
||||
size_t broadcast_max{INVENTORY_BROADCAST_TARGET + (tx_relay->m_tx_inventory_to_send.size()/1000)*5};
|
||||
broadcast_max = std::min<size_t>(INVENTORY_BROADCAST_MAX, broadcast_max);
|
||||
while (!vInvTx.empty() && nRelayedTransactions < broadcast_max) {
|
||||
// Fetch the top element from the heap
|
||||
std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder);
|
||||
@@ -5783,14 +5733,12 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
|
||||
if (!txinfo.tx) {
|
||||
continue;
|
||||
}
|
||||
auto txid = txinfo.tx->GetHash();
|
||||
// Peer told you to not send transactions at that feerate? Don't bother sending it.
|
||||
if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) {
|
||||
continue;
|
||||
}
|
||||
if (tx_relay->m_bloom_filter && !tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue;
|
||||
// Send
|
||||
tx_relay->m_recently_announced_invs.insert(hash);
|
||||
vInv.push_back(inv);
|
||||
nRelayedTransactions++;
|
||||
if (vInv.size() == MAX_INV_SZ) {
|
||||
@@ -5798,15 +5746,11 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
|
||||
vInv.clear();
|
||||
}
|
||||
tx_relay->m_tx_inventory_known_filter.insert(hash);
|
||||
if (hash != txid) {
|
||||
// Insert txid into m_tx_inventory_known_filter, even for
|
||||
// wtxidrelay peers. This prevents re-adding of
|
||||
// unconfirmed parents to the recently_announced
|
||||
// filter, when a child tx is requested. See
|
||||
// ProcessGetData().
|
||||
tx_relay->m_tx_inventory_known_filter.insert(txid);
|
||||
}
|
||||
}
|
||||
|
||||
// Ensure we'll respond to GETDATA requests for anything we've just announced
|
||||
LOCK(m_mempool.cs);
|
||||
tx_relay->m_last_inv_sequence = m_mempool.GetSequence();
|
||||
}
|
||||
}
|
||||
if (!vInv.empty())
|
||||
|
||||
Reference in New Issue
Block a user