💬 brunoerg commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1544020183)
@hebasto I got an error when trying to access [25.0 Draft Release Notes](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/25.0-Release-Notes-draft)

💬 sr-gi 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#issuecomment-1544020927)
> > 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. The client just cannot request extremely recent things, which will be announced normally every ~5 seconds. This is "I haven't even announced this to you yet, why are you requesting it from me? seems suspicious. Just wait 5 seconds and I'll announce it to you."
>
> A normal client, who presumably
...

💬 josibake commented on pull request "[24.1] Backports for rc3":
(https://github.com/bitcoin/bitcoin/pull/27614#issuecomment-1544027466)
ACK https://github.com/bitcoin/bitcoin/pull/27614/commits/97f5e28830dfa5786cdc5df62e245e03fa393e19

Verified that the commits added are the correct ones from the PRs linked, compiled and ran the tests. Document changes look good. Looks like the CI failures are just github being crappy

🤔 hebasto reviewed a pull request: "depends: no-longer nuke libc++abi.so* in native_clang package"
(https://github.com/bitcoin/bitcoin/pull/27493#pullrequestreview-1422646806)
ACK 9ae854da193f3c4bda38a75e96f9b989b289baab

Actually, this PR introduces no changes to the `native_clang` package at all:
```
$ git fetch origin pull/27493/head
$ git checkout FETCH_HEAD
$ make -C depends clean-all
$ make -C depends native_clang HOST=x86_64-apple-darwin
$ cat depends/built/x86_64-apple-darwin/native_clang/native_clang-10.0.1-*.tar.gz.hash
300b13fa3db495f3ad33f729a2fa2688e84d928a17ef8f4eb1164e062ad924c6 native_clang-10.0.1-a7086a951bd.tar.gz
$ git checkout HEAD~1
$
...

📝 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