💬 darosior commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#discussion_r1670149765)
Done.

👍 dergoegge approved a pull request: "random: add benchmarks and drop unnecessary Shuffle function"
(https://github.com/bitcoin/bitcoin/pull/30396#pullrequestreview-2165745179)
Code review ACK 6ecda04fefad980872c72fba89844393f5581120

👍 dergoegge approved a pull request: "refactor: use existing RNG object in ProcessGetBlockData"
(https://github.com/bitcoin/bitcoin/pull/30393#pullrequestreview-2165746582)
Code review ACK fa2e74879a3f7423c8d01aa1376b2bd9ccf5658d

💬 fanquake commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2217223685)
> ZMQ is trying to link against realtime on macOS.

Patched out all usage of `librt` here. Landed one related change upstream: https://github.com/zeromq/libzmq/pull/4702.

💬 fanquake commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2217224491)
Guix Build (aarch64):
```bash
4f3beefc6f4dc2a44829697ebd14e2f1016a35372f261bd379c66190e93db745 guix-build-f684b5d7ddfe/output/aarch64-linux-gnu/SHA256SUMS.part
b80ef38b4ad2a51bd033114884cb4b30e6b10675a6bfca08aa96680a7a754fc9 guix-build-f684b5d7ddfe/output/aarch64-linux-gnu/bitcoin-f684b5d7ddfe-aarch64-linux-gnu-debug.tar.gz
1c9c9198bda3419e9d49c809ef2a7e4e1c2b7846f9e03695e5175dbc7a5c0c8f guix-build-f684b5d7ddfe/output/aarch64-linux-gnu/bitcoin-f684b5d7ddfe-aarch64-linux-gnu.tar.gz
3a9b4b
...

💬 dergoegge commented on issue "fuzz: mini_miner_selection: ASSERT: mock_template_txids.size() <= blocktemplate->block.vtx.size()":
(https://github.com/bitcoin/bitcoin/issues/30367#issuecomment-2217242177)
cc @glozow @murchandamus (in case you missed it) would be nice to have this fixed and also have an explanation of the actual bug, since it is somewhat of a mystery why it's so hard to find

💬 theStack commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1670235553)
Makes sense. I think the easiest way to achieve this is by passing the `CNode` reference as `const` instead. Since that is simple enough, I've added an extra commit for this (with an example of a compiler error in the commit message).

💬 theStack commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1670236195)
Thanks, I've decided to take the second approach, i.e. guarding with `NetEventsInterface::g_msgproc_mutex`.

👍 dergoegge approved a pull request: "net: fix race condition in self-connect detection"
(https://github.com/bitcoin/bitcoin/pull/30394#pullrequestreview-2165854678)
Code review ACK fb587ce36afcc8789214af45a36082c4f5238e50

💬 fjahr commented on pull request "rpc, rest: Improve getblock error when only header is available":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1670347496)
Here and in `rest.cpp`: Structuring it like this should save us the `IsBlockPruned()` call in the normal case (untested). `IsBlockPruned()` checks the same condition plus something extra so this should not change behavior.

```suggestion
if (!(blockindex.nStatus & BLOCK_HAVE_DATA)) {
if (blockman.IsBlockPruned(blockindex) {
throw JSONRPCError(RPC_MISC_ERROR, "Block not available (pruned data)");
}
throw JSONRPCError(RPC_MISC_ERRO
...

💬 fjahr commented on pull request "rpc, rest: Improve getblock error when only header is available":
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2217409014)
Concept ACK

🤔 paplorinc reviewed a pull request: "Don't empty dbcache on prune flushes: >30% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/28280#pullrequestreview-2165763930)
I think we can remove some of the cursor's state, otherwise I'm mostly OK with the change, thanks for your perseverance!

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670186109)
you basically reverting https://github.com/bitcoin/bitcoin/commit/2e16054a66b0576ec93d4032d6b74f0930a44fef here, right?
Was https://github.com/bitcoin/bitcoin/pull/17487#discussion_r1087087728 just a temporary sanity check that we don't need anymore?

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670195903)
nit: if we keep it, is `curr` an commonly used abbreviation or could we expand it?

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670199990)
nit: are you deliberately keeping the original `&` placements when refactoring, or when do you use `X& y` and when [`X &y`](https://github.com/bitcoin/bitcoin/pull/28280/commits/adcccaae6b67e0ac574eaab16842bed1ba15389f#diff-095ce1081a930998a10b37358fae5499ac47f8cb6f25f5df5d88e920a54e0341R285)?

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670230733)
nit: to be sure that the `Next` method is indeed a `noexcept`, we could annotate https://github.com/bitcoin/bitcoin/blob/master/src/coins.cpp#L47 as well (it seems to be `noexcept` based on the impl), so maybe in a followup PR we could do that as well

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670252649)
nit: we have a few of extra parenthesis now
```suggestion
fresh = !it->second.IsDirty();
```

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670340407)
nit: now that you've extracted the first complex parameter, it might make the code slightly more readable if we extracted the second one as well

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670224611)
is this important enough to cover it with a test?

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670344719)
nit: does the update have to be in the middle or can it go to the top, like the rest (the related tests pass that way)?