Currently, non-announced transactions can be served after a BIP35 MEMPOOl message has been received from a peer. By following this pattern, we are potentially allowing peers to fingerprint transactions that may have originated in our node and to perform timing analysis on the propagation of transactions that enter our mempool.
This simply removes the relay bypass so only announced transactions are served to peers.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | glozow, willcl-ark |
| Approach NACK | vasild |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Motivation: this fix was discussed over IRC by @willcl-ark @glozow @sipa and @darosior
5632@@ -5639,7 +5633,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
5633 if (!tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue;
5634 }
5635 tx_relay->m_tx_inventory_known_filter.insert(hash);
5636- // Responses to MEMPOOL requests bypass the m_recently_announced_invs filter.
5637+ tx_relay->m_recently_announced_invs.insert(hash);
GETDATA txid to FILTERCLEAR; FILTERADD whatever; MEMPOOL ? In which case we’re just as easily fingerprinted, but we spend more cpu while being fingerprinted?
Do you mean keep adding what you’re interested in into the filter so it triggers an INV from the node and, therefore, we can request it (or, similarly, keep resetting the filter and just add the one single transaction that we want to probe)? If so, isn’t that equivalent to having no filter?
I think in both of the aforementioned cases (which boil down to the same logic if I’m not mistaken) the patch prevents the attacker from requesting unannounced transactions straight-away, they will have to wait for m_next_inv_send_time to go off.
Ah, I see what you mean. That makes sense.
Next point on those lines though: adding many things to m_recently_announced_invs can overflow the bloom filter, either causing false positives (leading to privacy bugs), or since it’s a rolling bloom filter, causing overflow leading to false negatives (leading to functionality bugs where you can’t get the txs from the MEMPOOL response). Wouldn’t it be better to make the insertion conditional on txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY ? Even then this approach seems like it can trigger false positives fairly easily, since the rolling bloom filter capacity is only 3500 txs?
Wouldn’t it be better to make the insertion conditional on
txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY?
Yeah, I think that makes perfect sense, given there is no point in adding something to the filter if we are going to unconditionally serve it anyway.
Even then this approach seems like it can trigger false positives fairly easily, since the rolling bloom filter capacity is only 3500 txs?
It may indeed. I haven’t got my head around how to properly compute what are the odds of this happening though, given it feels like it depends on how often MEMPOOL messages are sent to us, which may be unbounded (how many MEMPOOL requests can we receive in a UNCONDITIONAL_RELAY_DELAY period, and how full our mempool may be?). In any case, this feels detrimental for the peer implementing this behavior instead of for us (?)
Something I’m really wondering is why this is actually bound by INVENTORY_MAX_RECENT_RELAY (3500) instead of by MAX_PEER_TX_ANNOUNCEMENTS (5000). It feels quite inconsistent that we are able to accept up to 5000 transaction announcements from someone but we are only able to remember the last 3500 that we have announced to someone(else).
I think what may make even more sense would be to reply to mempool messages only with transactions that fall into the txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY check, that way we don’t really have to add transactions to m_recently_announced_invs here, and we are also discouraging peers bloating us with MEMPOOL messages. As long as they have a filter set, any transaction that matches their filter will be announced to them anyway (h/t @sdaftuar). I guess this could be done as a followup if it makes sense.
Wouldn’t it be better to make the insertion conditional on
txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY?
Also wouldn’t that also apply here? https://github.com/bitcoin/bitcoin/blob/27c40e3dcf280a137bcfaa787674c26781d911b8/src/net_processing.cpp#L5688
MAX_PEER_TX_ANNOUNCEMENTS doesn’t really provide a rate limit – it stops the queue from being more than 5k at any point in time, but the flow through the queue could still be 400tx/s, for instance. Beyond that, m_recently_announced_invs only tracks things we announced, not things we received, and we rate limit that (as per the static_assert on INVENTORY_MAX_RECENT_RELAY).
Delaying MEMPOOL results for 2 minutes seems annoying. But how about just recording the timestamp of the last MEMPOOL request into the txrelay structure, and skipping the bloom lookup if m_time <= max(now-2min, txrelay->last_mempool_request) ?
You could make the inv-trickle insertion into the bloom filter conditional too, but I think that would so rarely actually have an effect that it wouldn’t matter?
But how about just recording the timestamp of the last MEMPOOL request …
Patch might look something like… (EDIT: Err, except that’s just essentially reverting back to what we have now?)
0--- a/src/net_processing.cpp
1+++ b/src/net_processing.cpp
2@@ -295,6 +295,8 @@ struct Peer {
3 * permitted if the peer has NetPermissionFlags::Mempool or we advertise
4 * NODE_BLOOM. See BIP35. */
5 bool m_send_mempool GUARDED_BY(m_tx_inventory_mutex){false};
6+ std::chrono::microseconds m_last_mempool_time GUARDED_BY(m_tx_inventory_mutex){0};
7+
8 /** The next time after which we will send an `inv` message containing
9 * transaction announcements to this peer. */
10 std::chrono::microseconds m_next_inv_send_time GUARDED_BY(m_tx_inventory_mutex){0};
11@@ -2258,7 +2260,7 @@ CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer::TxRelay& tx_relay,
12 auto txinfo = m_mempool.info(gtxid);
13 if (txinfo.tx) {
14 // If a TX is older than UNCONDITIONAL_RELAY_DELAY, permit the request unconditionally.
15- if (txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY) {
16+ if (txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY || WITH_LOCK(tx_relay.m_tx_inventory_mutex, return txinfo.m_time <= tx_relay.m_last_mempool_time)) {
17 return std::move(txinfo.tx);
18 }
19 }
20@@ -5619,6 +5621,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
21 tx_relay->m_send_mempool = false;
22 const CFeeRate filterrate{tx_relay->m_fee_filter_received.load()};
23
24+ tx_relay->m_last_mempool_time = current_time;
25 LOCK(tx_relay->m_bloom_filter_mutex);
26
27 for (const auto& txinfo : vtxinfo) {
28@@ -5632,7 +5635,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
29 if (tx_relay->m_bloom_filter) {
30 if (!tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue;
31 }
32- tx_relay->m_tx_inventory_known_filter.insert(hash);
33 tx_relay->m_recently_announced_invs.insert(hash);
34 vInv.push_back(inv);
35 if (vInv.size() == MAX_INV_SZ) {
how about just making sure you only send at most 3500 mempool entries younger than 2 minutes, in response to a MEMPOOL message
Wouldn’t that imply that peers are now expected to send the mempool message in a loop (or at least twice over a span of two minutes) to detect if there were more transactions to get? If yes, I wonder if it conflicts with the idea to potentially only allow a single mempool message per connection.
Peer, knowing that only transaction in the mempool can be in them.
I wonder if we can remove mapRelay and as a result can get more efficient data structures to remember (w)txids per Peer, knowing that only transaction in the mempool can be in them.
mapRelay is mostly what’s in the mempool, but can also contain entries that were announced less than 15min ago + fell out of the mempool since then. This is sometimes good for fingerprinting :shrug: but also seems like largely redundant information to me.
I don’t think we can remove mapRelay – we need it so we can continue to relay recently announced txs that we’ve removed from the mempool?
How about this approach: when we get a MEMPOOL message, at the next fSendTrickle time, for each tx that matches the filter, if it’s older than 2 minutes, announce it immediately, otherwise just put it into m_tx_inventory_to_send and let it be sent out at the usual rate. Only use m_recently_announced_invs for txs relayed via m_tx_inventory_to_send. (Maybe bump the “2 minutes” by a small random amount)
This is sometimes good for fingerprinting
fingerprinting seems bad, so another reason for removal?
I don’t think we can remove mapRelay – we need it so we can continue to relay recently announced txs that we’ve removed from the mempool?
Sure that is what it does, but if the only use case is for spy nodes to fingerprint, it may be better to remove it?
See #27625. Feel free to NACK.
This is sometimes good for fingerprinting
fingerprinting seems bad, so another reason for removal?
Sorry I meant that it’s sometimes good for preventing fingerprinting. In the replacement scenario which @ajtowns is describing. Timing of entry of replacement == notfound of replaced tx. With mapRelay, since you have the replaced tx for 15 minutes, you’ll still serve it instead of immediately saying notfound. But mapRelay is useless against this if the spy node just waits 15 minutes before doing the replacement. So this could just be a lost cause.
Anyway, will take this to #27625 instead.
mapRelay reveals a secret – that’s 15minutes after it was added, but the time it was added is the time it was first announced (probably to an outbound peer), which is already randomised/batched, compared to the time we received it?
What I mean is Leak available without mapRelay:
^This is prevented with mapRelay because after txA leaves mempool, we’ll still be able to serve it from mapRelay.
This is still possible with mapRelay:
Even then this approach seems like it can trigger false positives fairly easily, since the rolling bloom filter capacity is only 3500 txs?
I didn’t understand this. How is this different from a peer connecting and waiting for 3500 “normal” newly announced transaction to be announced and added to the bloom filter to achieve the same false positive rate?
or since it’s a rolling bloom filter, causing overflow leading to false negatives (leading to functionality bugs where you can’t get the txs from the MEMPOOL response).
Same here. How is this different from a “normal” sustained spike in tx announcements on the network?
I guess we’d need to bypass the bloom filter for fSendTrickle=always-true peers, but that seems unrelated from this pull, unless there is a change so that the mempool permission also forces fSendTrickle to true?
I think those are rate limited so they don’t overflow in less than two minutes, after which they are not relevant anymore.
For outbound connections, INVs are sent out every 2s (OUTBOUND_INV_BROADCAST_INTERVAL), while for inbound, they are sent every 5s (OUTBOUND_INV_BROADCAST_INTERVAL). In a two-minute timespan, that results in 2100 and 840 INVs at max1, respectively. The m_recently_announced_invs rolling filter is only checked in FindTxForGetData after making sure the traction cannot be sent unconditionally (i.e. after 2min), so if the filter overflows after that it’s not an issue.
1 data is also added to m_recently_announced_invs when relaying a transaction with a recent but unconfirmed parent (https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L2343). I’m unsure if that is rate-limited at all, or if the remaining 1400 slots (in the worse case) are to account for that not overflowing.
The scenario here is:
In normal relay, over the course of two minutes we’d only advertise ~840 txs (~2100 for an outbound), and only add the txs to the bloom filter when advertised, so the rollover would only happen very rarely (less than 1-in-a-million times for an outbound, fewer for an inbound). After #27610 we’ll relay more txs during spikes, so these error rates can increase, though.
mempool message was rate-limited and size-limited. (Which on a second thought, probably doesn’t make sense to do). So a single reply can hold up to 50'000 inv entries, which will certainly overflow. Sorry for the distraction.
Side note: The 50k max inv size also seems to overflow, which is probably another source for false negatives, depending on the use case.
Image stolen from the comment #27426 (comment) by 0xB10C .
Side note: The 50k max inv size also seems to overflow, which is probably another source for false negatives, depending on the use case.
You lost me there. Do you mean this may be a possible source of false negatives for the current path, or for both?
In this patch, I can see that potentially being an issue (that’s what the original concern from AJ’s comes from), but I don’t see how that is an issue for master, given nothing is added to m_recently_announced_invs when building this huge INVs as a result of a MEMPOOL message.
You lost me there.
Apologies, I typed something incorrect, again.
2265- // unconditionally.
2266- if ((mempool_req.count() && txinfo.m_time <= mempool_req) || txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY) {
2267- return std::move(txinfo.tx);
2268- }
2269+ // If a TX is older than UNCONDITIONAL_RELAY_DELAY, permit the request unconditionally.
2270+ if (txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY) return std::move(txinfo.tx);
m_last_mempool_req is now unused and can be removed completely? Also, it might be good to not touch the return here when there is no reason, to avoid code churn and review burden.
m_last_mempool_req is now unused and can be removed completely?
Looks like it
Also, it might be good to not touch the return here when there is no reason, to avoid code churn and review burden.
I thought the only reason why this was split into two lines is that it was a bit too long (?), I tried to follow the same approach as in L2267 and, while I think it looks more compact the way it is now, I don’t have a strong opinion about on styling atm, so I’m happy to amend it if there is consensus that this is best.
Concept ACK
The issue on master that I believe this addresses is being able to learn exactly when a transaction enters your mempool. You send mempool and get the response, then put out the tx, then getdata getdata getdata getdata... until the node tells you the tx instead of notfound. Also in general I think it’s quite sensible to only serve transactions you announced.
I’m only a casual lurker on the net_processing side but i was under the impression we were thinking of the BIP35 as permissioned anyways. If we are already trusting the peer that sends use MEMPOOL message, why not just include everything we’ve got?
A practical counter-argument to this could be - yeah but at this time around 20% to 30% of reachable nodes set peerbloomfilters so it’s still worth closing an obvious fingerprinting vector.
Another thing is whether it’s breaking any usecase. Since the message is from a trusted peer a client may be relying on getting the whole mempool. For instance BTCPay/NBXplorer needs whitelist=mempool iirc. What’s the proportion of announced vs non-announced transactions? How much would be left out with this patch?
i was under the impression we were thinking of the BIP35 as permissioned anyways. … For instance BTCPay/NBXplorer needs
whitelist=mempooliirc.
Just on these points, currently the MEMPOOL message will be responded to either if you have the whitelist permission mempool on the peer or if you have enabled NODE_BLOOM (with -peerbloomfilters). In the latter case I don’t think we can accurately describe these as “trusted peers”. See the changes in 4581a68 (#27559) which clarify this.
MEMPOOL messages, but it does not do that. Before and after the PR we would send the full mempool in a reply to mempool (correct me if I got it wrong).
What’s the proportion of announced vs non-announced transactions? How much would be left out with this patch?
The PR (as it is right now) still announces everything in response to mempool. This is saying “I haven’t even announced this to you, why are you requesting it from me?” A normal client, who presumably does not already know what it wants to request until it receives an announcement (i.e. using mempool to learn about transactions), shouldn’t be affected at all.
161+ self.log.info("Create a third transaction (that was therefore not part of the mempool message) and try to retrieve it")
162+ add_txid, _ = self.wallet.send_to(from_node=self.nodes[0], scriptPubKey=getnewdestination()[1], amount=3 * COIN)
163+ filter_peer.send_and_ping(msg_getdata([CInv(t=MSG_WTX, h=int(self.nodes[0].getmempoolentry(add_txid)["wtxid"], 16))]))
164+ assert not filter_peer.tx_received
165+
166+ self.log.info("Check this is also sound for a peer that has a match-all filter set")
0 self.log.info("Check all transactions are returned by the match-all filter")
166+ self.log.info("Check this is also sound for a peer that has a match-all filter set")
167+ unfilter_peer = P2PBloomFilter()
168+ self.nodes[0].add_p2p_connection(unfilter_peer)
169+ unfilter_peer.send_and_ping(msg_mempool())
170+
171+ self.log.info("Here the all transactions are relevant")
Is it the case that this PR will change the behavior in the following two cases (from the PR title and description I got the impression that it is wider scope):
MEMPOOL request and replies to itMEMPOOL message)GETDATA for that transactionIn master it would send the transaction even though it never advertised it with INV due to the txinfo.m_time <= mempool_req check (both values would be equal).
In this PR it would claim “not found”.
This can be resolved by increasing the precision of TxRelay::m_last_mempool_req and TxMempoolInfo::m_time or by changing the condition to <.
MEMPOOL request and replies to it, but some transaction is omitted from the reply because it is not in TxRelay::m_bloom_filter.GETDATA for that transactionIn master it would send the transaction even though it never advertised it with INV because PeerManagerImpl::FindTxForGetData() does not check the bloom filter.
In this PR it would claim “not found”.
Is this a problem at all? If yes, then it can be resolved by checking the bloom filter from PeerManagerImpl::FindTxForGetData().
I’m only a casual lurker on the net_processing side but i was under the impression we were thinking of the BIP35 as permissioned anyways. If we are already trusting the peer that sends use MEMPOOL message, why not just include everything we’ve got? A practical counter-argument to this could be - yeah but at this time around 20% to 30% of reachable nodes set
peerbloomfiltersso it’s still worth closing an obvious fingerprinting vector.
We still do. This is not modifying the behavior of the announcements that result from a mempool message, but the behavior of the node to the getdata messages that may follow. Before the patch, a node was able to send us a mempool message and, from that point on, request any transaction from our mempool (even if we haven’t announced that to them at all).
Another thing is whether it’s breaking any usecase. Since the message is from a trusted peer a client may be relying on getting the whole mempool. For instance BTCPay/NBXplorer needs
whitelist=mempooliirc. What’s the proportion of announced vs non-announced transactions? How much would be left out with this patch?
As of 27c40e3 we still do, even though I would argue that, by doing this, we are encouraging peers to spam us with mempool messages instead of waiting for us to announce the transactions that match their filters in due time.
The title “avoid serving non-announced txs as a result of a MEMPOOL message” is confusing - it gives the impression that this PR will change the response to
MEMPOOLmessages, but it does not do that. Before and after the PR we would send the full mempool in a reply tomempool(correct me if I got it wrong).
Maybe it should read “after receiving a MEMPOOL message” instead of “as a result of a MEMPOOL message”.
What this PR is trying to achieve is to prevent us from responding to getdata requests about unannounced transactions after receiving a mempool message.
Is it the case that this PR will change the behavior in the following two cases (from the PR title and description I got the impression that it is wider scope):
Case 1
- the node receives
MEMPOOLrequest and replies to it- in the same second a new transaction enters the mempool (after the
MEMPOOLmessage)- the node receives
GETDATAfor that transactionIn
masterit would send the transaction even though it never advertised it withINVdue to thetxinfo.m_time <= mempool_reqcheck (both values would be equal).In this PR it would claim “not found”.
This can be resolved by increasing the precision of
TxRelay::m_last_mempool_reqandTxMempoolInfo::m_timeor by changing the condition to<.
This is exactly what we are trying to prevent, but I don’t think your approach fixes the issue. This condition is checking that our inv (txinfo) is more recent than the last mempool_req, meaning that once a mempool request has been processed the peer can request any transaction from our mempool. Why is this bad? It makes transactions that enter our memopool easily probable. The node can create a transaction (tx), send it through any other link, and spam us with getdata messages until we reply with it.
Case 2
- the node receives
MEMPOOLrequest and replies to it, but some transaction is omitted from the reply because it is not inTxRelay::m_bloom_filter.- the node receives
GETDATAfor that transactionIn
masterit would send the transaction even though it never advertised it withINVbecausePeerManagerImpl::FindTxForGetData()does not check the bloom filter.In this PR it would claim “not found”.
Is this a problem at all? If yes, then it can be resolved by checking the bloom filter from
PeerManagerImpl::FindTxForGetData().
Arguably this is the expected behavior, master’s is not.
The response to the mempool message didn’t include an INV from that specific transaction (both in the patch nor in master’s) because it didn’t match the filter set by the peer, therefore, the peer should not be interested in it. With respect to why claiming not found after the peer requests the transaction (what makes this different from master) I think we can default to the same reasoning as per Case 1.
txinfo.m_time <= mempool_req
This condition is checking that our inv (
txinfo) is more recent than the lastmempool_req, meaning that once a mempool request has been processed the peer can request any transaction from our mempool.
I don’t get it. Is it not that the condition is checking whether txinfo is older than the last mempool_req? In that case the transaction has been part of the MEMPOOL reply, so we have sent an INV about it.
txinfo.m_time <= mempool_reqThis condition is checking that our inv (
txinfo) is more recent than the lastmempool_req, meaning that once a mempool request has been processed the peer can request any transaction from our mempool.I don’t get it. Is it not that the condition is checking whether
txinfois older than the lastmempool_req? In that case the transaction has been part of theMEMPOOLreply, so we have sent anINVabout it.
Ah, I think you’re right! So this changes Case 2, not Case 1.
txinfo.m_time <= mempool_reqThis condition is checking that our inv (
txinfo) is more recent than the lastmempool_req, meaning that once a mempool request has been processed the peer can request any transaction from our mempool.I don’t get it. Is it not that the condition is checking whether
txinfois older than the lastmempool_req? In that case the transaction has been part of theMEMPOOLreply, so we have sent anINVabout it.Ah, I think you’re right! So this changes Case 2, not Case 1.
He is indeed. The test for Case 1 was passing because both requests were processed within the same second 😅
No, wait! This PR changes both Case1 and Case2, but Case1 is only about things happening in the same second.
What I tried yesterday:
net_processing.cpp changes, both tests Request the irrelevant transaction even though it has not being offered to us and Create a third transaction (that was therefore not part of the mempool message) and try to... failed. Good! To reach the second test I disabled the assert from the first.But I couldn’t explain why the second test would fail (without net_processing.cpp changes aka on master). Then I suspected things happen in the same second in the test and added time.sleep(1) before creating the third transaction and then the test passed.
Concept ACK. I agree that we should not respond to GETDATA asking for a transaction which we did not INV to that peer and this PR does that.
However, I agree with @ajtowns’s concern raised above about blowing up m_recently_announced_invs. That bloom filter has size 3500 and if we put 300k elements in it, then it is probably not going to work as expected.
Approach NACK due to that.
So, to achieve the goal of not replying to GETDATA without a prior INV we have to remember all INVs sent to the peer, but that is kind of “hard” considering a MEMPOOL reply sends 300k INVs…
CRollingBloomFilter if m_recently_announced_invs is changed from 3500 to 300000 then its size would increase from 37KB to 3MB. I guess that is too much and maybe was the reason why MEMPOOL replies don’t use m_recently_announced_invs and instead use the m_last_mempool_req timing mechanism.
No, wait! This PR changes both Case1 and Case2, but Case1 is only about things happening in the same second.
I meant regarding the time checks in master, the checks were indeed protecting against querying something that was in the mempool before the mempool message was received. Meaning that while this patch fixes the issues around requesting data after a mempool message is received, they are not as severe as initially assumed.
However, I agree with @ajtowns’s concern raised above about blowing up
m_recently_announced_invs. That bloom filter has size 3500 and if we put 300k elements in it, then it is probably not going to work as expected. Approach NACK due to that.
I think at this point there are two easy solutions to the problem (with their corresponding tradeoffs, of course):
Do things as stated in the patch but guarding addition to the m_recently_announced_invs rolling filter by checking that the data we are adding is not older than two minutes, as @ajtowns originally suggested. This will prevent us from adding data that will be served unconditionally anyway, reducing the odds of overflowing the filter. However, I’m not sure how AJ computed the odds of the filter overflowing due to high traffic, so I’m not able to convince myself this is enough.
Keep the m_recently_announced_invs bypass as it is in master, but do check the bloom filter when deciding whether to relay something that may have been announced to a peer as a response to a mempool message. This has two main drawbacks:
MEMPOOL message (https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L5649-L5651) and again as part of the combined check in FindTxForGetData (https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L2271).MEMPOOL message and before requesting the data and we will end up serving something we haven’t really announced. However, I cannot think of a proper use case (or attack, FWIW) that takes advantage of this in a meaningful way. If the goal was for us to include all the data we had in our mempool when sending us the MEMPOOL message, they could have had a clear filter to begin with.
MEMPOOL message and the second GETDATA message are not processed within the same second, as per @vasild comment.
m_recently_announced_invs conditionally to them not being unconditionally relayable and rebases master.
Currently, non-announced transactions can be served after a BIP35 MEMPOOl
message has been received from a peer. By following this pattern, we are
potentially allowing peers to fingerprint transactions that may have
originated in our node and to perform timing analysis on the propagation
of transactions that enter our mempool.
This simply removes the relay bypass so only announced transactions are
served to peers.
🐙 This pull request conflicts with the target branch and needs rebase.
noprivacy permission flag…
Maybe the test? Though, I haven’t looked at it recently.
That may make sense. I’ll take a look and rebase
Maybe the test? Though, I haven’t looked at it recently.
The test can be adapted to fit the logic after #27675, but the patch itself does not seem relevant anymore. Would it be worth reworking this PR or just closing it and opening one for the test?