📝 MarcoFalke opened a pull request: "p2p: Stop relaying non-mempool txs"
(https://github.com/bitcoin/bitcoin/pull/27625)
`mapRelay` (used to relay announced transactions that are no longer in the mempool) has issues:

* It doesn't have an absolute memory limit, only an implicit one based on the rate of transaction announcements
* It doesn't have a use-case, and can be used to spy (see below)

Fix all issues by removing `mapRelay`.

For more context, on why a transaction may have been removed from the mempool, see https://github.com/bitcoin/bitcoin/blob/c2f2abd0a4f4bd18bfca41b632d21d803479f3f4/src/txmempool.
...

💬 glozow commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1191209626)
I've written a unit test here: https://github.com/glozow/bitcoin/commits/2023-05-comparedepthandscore-test

🤔 jonatack reviewed a pull request: "p2p: Allow whitelisting outgoing connections"
(https://github.com/bitcoin/bitcoin/pull/27114#pullrequestreview-1422614006)
Approach ACK, modulo comments and https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186378760 -- it might be better for that commit to be a pure refactoring. Thanks for addressing the previous feedback.

💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191201370)
Why this change?

💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191183121)
0a77e6ab37a9b23bd293 I would suggest placing this new helper with the other ones standalone ones, i.e. after `IsLocal()` or higher, not between the `CConnman::FindNode()` class members (unless there's a reason that I didn't see).

💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191191757)
ea46500ece2
```suggestion
"Additional flags \"in\" and \"out\" control whether permissions apply to incoming connections and/or outgoing (default: %s). "
```

💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1191197012)
f99912850e2a50b1a
```suggestion
# Whitelist peers to speed up tx relay / mempool sync. Don't use if testing tx relay or timing.
```

💬 MarcoFalke commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1191211092)
> 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.

🤔 MarcoFalke reviewed a pull request: "Improve performance of p2p inv to send queues"
(https://github.com/bitcoin/bitcoin/pull/27610#pullrequestreview-1422225046)
first commit lgtm. Left one question. Didn't look too long at the second commit.

💬 MarcoFalke commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1190938459)
question (feel free to ignore): I tried to figure out if there was a reason why this was re-ordered, but couldn't find one. An equivalent patch of simply changing the literals `false` to `true` and `true` to `false` in this function should have achieved the same, right?

💬 ajtowns commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1191220466)
Knowing when a tx is removed from the mempool means you know exactly when the tx that replaced it was added to the mempool... Txs are also removed from the mempool when a block comes in; if we process the block first, then see a GETDATA for a tx in the block, replying with the tx is better for block propagation (including compact blocks) that sending a NOTFOUND...

💬 sdaftuar commented on pull request "p2p: Stop relaying non-mempool txs":
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1544047433)
I think the removed-for-block case is the main one we care about -- in a situation where a peer has requested a transaction where we have received a block containing the transaction in the time between announcement and relay, it seems likely that the peer is about to receive the block as well, and with the compact block protocol we may be helping our peer quite a bit (potentially saving a round trip) to deliver the transaction before the block arrives to them.

📝 instagibbs opened a pull request: "Parallel compact block downloads, take 3"
(https://github.com/bitcoin/bitcoin/pull/27626)
This is an attempt at mitigating https://github.com/bitcoin/bitcoin/issues/25258 , which is a revival of https://github.com/bitcoin/bitcoin/pull/10984, which is a revival of https://github.com/bitcoin/bitcoin/pull/9447.

This PR attempts to mitigate a single case, where high bandwidth peers can bail us out of a flakey
peer not completing blocks for us. We allow up to 2 additional getblocktxns requests per unique block.
This would hopefully allow the chance for an honest high bandwidth peer t
...

💬 hebasto commented on pull request "kernel: Remove args, chainparams, chainparamsbase from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27576#issuecomment-1544060017)
Concept ACK.

💬 ajtowns commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1191235815)
The `CompareTxMemPoolEntryByScore` comparator returns `false` on equality (ie, it's "less than" not "less than or equal"), so this preserves that behaviour.

📝 MarcoFalke converted_to_draft a pull request: "p2p: Stop relaying non-mempool txs"
(https://github.com/bitcoin/bitcoin/pull/27625)
`mapRelay` (used to relay announced transactions that are no longer in the mempool) has issues:

* It doesn't have an absolute memory limit, only an implicit one based on the rate of transaction announcements
* It doesn't have a use-case, and can be used to spy (see below)

Fix all issues by removing `mapRelay`.

For more context, on why a transaction may have been removed from the mempool, see https://github.com/bitcoin/bitcoin/blob/c2f2abd0a4f4bd18bfca41b632d21d803479f3f4/src/txmempool.
...

🤔 Sjors reviewed a pull request: "p2p: Avoid prematurely clearing download state for other peers"
(https://github.com/bitcoin/bitcoin/pull/27608#pullrequestreview-1422180036)
utACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4

💬 Sjors commented on pull request "p2p: Avoid prematurely clearing download state for other peers":
(https://github.com/bitcoin/bitcoin/pull/27608#discussion_r1190909435)
This comment could be improved:

```cpp
// When processing block related messages from our peers,
// we treat requested blocks different from unsolicited ones.
// When making this distinction we only keep track of the
// last peer we requested from.
```

💬 Sjors commented on pull request "p2p: Avoid prematurely clearing download state for other peers":
(https://github.com/bitcoin/bitcoin/pull/27608#discussion_r1191267648)
Can't we have a race condition where the block is not `new`? In any case this function is extremely fast if called twice.

🤔 glozow reviewed a pull request: "[24.1] Backports for rc3"
(https://github.com/bitcoin/bitcoin/pull/27614#pullrequestreview-1422778545)
ACK 97f5e28830dfa5786cdc5df62e245e03fa393e19