💬 stickies-v commented on pull request "add ryanofsky to trusted-keys":
(https://github.com/bitcoin/bitcoin/pull/27604#issuecomment-1543798949)
> The fact that I'm the only one here stating the obvious (...) is really scary to me.

Not to speak for anyone else, but perhaps the lack of engagement on your messages is because most regular contributors are just generally happy with the maintainer decisions and don't agree with your assessment but do not wish to keep discussing the same thing time and again?

💬 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_r1191023800)
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.

💬 stickies-v commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1543830285)
With #27468 merged, I don't really see the need for 8ef940d39bfa7bc4ba867b70495b68c507297694 anymore?

💬 fanquake commented on pull request "build: LLVM 15 & LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1543831532)
> Shall I go ahead and PR that commit separately?

@theuni sgtm. I've got some related compile flag changes in #27493, but I'm going to just combine most of the relevant changes here. If we get the LLD switchover done for 26.x, then we can skip another bump in the interim.

For now, I've rebased this PR on top of all of your other commits above, and reverted back to LLVM 15.0.6 in depends. Also updated the PR description.

💬 michaelfolkson commented on pull request "add ryanofsky to trusted-keys":
(https://github.com/bitcoin/bitcoin/pull/27604#issuecomment-1543835912)
> Not to speak for anyone else, but perhaps the lack of engagement on your messages is because most regular contributors are just generally happy with the maintainer decisions and don't agree with your assessment but do not wish to keep discussing the same thing time and again?

It is a failure of communication and process around both CTV (which could have resulted in a chain split disrupting a hundred(s) billion dollar ecosystem) and now the addition, rejection and removal of maintainers. If
...

💬 theStack commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1191062084)
This field is set in `GetPrioritisedTransaction`, but not read anywhere. Was there a plan to also return that in `getprioritisedtransactions` or is there any other future use-case? If not, I think it can be removed.

💬 vasild 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-1543917289)
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 `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).

💬 jonatack commented on pull request "net, refactor: extract Network and BIP155Network logic to node/network":
(https://github.com/bitcoin/bitcoin/pull/27385#issuecomment-1543922356)
Rebased ⚡

💬 jonatack commented on pull request "test: move remaining rand code from util/setup_common to util/random":
(https://github.com/bitcoin/bitcoin/pull/27425#issuecomment-1543923414)
Rebased 🧸

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1543937304)
> With #27468 merged, I don't really see the need for [8ef940d](https://github.com/bitcoin/bitcoin/commit/8ef940d39bfa7bc4ba867b70495b68c507297694) anymore? Also the PR title and description need updating, this is not a bugfix PR anymore.

Updated them, thanks!

🤔 glozow reviewed a pull request: "Improve performance of p2p inv to send queues"
(https://github.com/bitcoin/bitcoin/pull/27610#pullrequestreview-1422536495)
code review ACK 5b3406094f2679dfb3763de4414257268565b943

👍 dergoegge approved a pull request: "Improve performance of p2p inv to send queues"
(https://github.com/bitcoin/bitcoin/pull/27610#pullrequestreview-1422347722)
utACK 5b3406094f2679dfb3763de4414257268565b943

The code and approach looks good to me.

(my comments can be addressed in a follow-up)

💬 dergoegge commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1191017253)
* `INVENTORY_BROADCAST_MAX` should be renamed or have its comment amended
* clang-format?
* Maybe add the commit description as a comment here

💬 dergoegge commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1191018175)
Explaining that in a comment would be nice

💬 dergoegge commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1191140520)
```suggestion
size_t broadcast_max{INVENTORY_BROADCAST_MAX + (tx_relay->m_tx_inventory_to_send.size() / 1000) * count_seconds(INBOUND_INVENTORY_BROADCAST_INTERVAL)};
```

💬 martinus commented on pull request "bench: Benchmark all `SHA256` implementations that are available on the system":
(https://github.com/bitcoin/bitcoin/pull/27598#discussion_r1191134489)
Hm, I don't really like that all of that SHA256 specific code is becoming part of the benchmark framework. Wouldn't it be sufficient to just adapt the SHA benchmark in `crypto_hash.cpp` so that there is a separate test for each of the implementations? There could be a separate benchmark for each value of the `enum UseImplementation`.

💬 martinus commented on pull request "bench: Benchmark all `SHA256` implementations that are available on the system":
(https://github.com/bitcoin/bitcoin/pull/27598#discussion_r1191135383)
This always prints a SHA log even when no SHA related benchmark is run

💬 glozow 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-1543955685)
> 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 does not
...

💬 glozow 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_r1191157352)
> 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.

💬 hebasto commented on pull request "cleanse: switch to SecureZeroMemory for Windows cross-compile, check for usage":
(https://github.com/bitcoin/bitcoin/pull/26950#issuecomment-1543981739)
Concept ACK.

💬 instagibbs commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1191169420)
static asserts or something would be even better