💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573685509)
In commit "Rewrite removeForReorg to avoid using sets"

Nit: given that you're rewriting most of this function anyway, make this variable name match style?

💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573704140)
In commit "doc: Add design notes for cluster mempool and explain new mempool limits"

Writing nit: `ie,` -> `i.e.,`

Also in two other places below.

💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573747087)
In commit "doc: Add design notes for cluster mempool and explain new mempool limits"

Nice write-up, enough to understand the terminology used in the codebase, but not going to detailed into the rationale.

💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573755332)
In commit "doc: add release notes snippet for cluster mempool"

Add "any combination of"? It's noted elsewhere, but the release notes are likely read by far more people.

💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573752182)
In commit "doc: add release notes snippet for cluster mempool"

I wouldn't say "better performance". While it's true many things are more performant now, that's not the motivation, and in many cases, not really critical anyway. How about "better decision-making"?

💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573756538)
In commit "doc: add release notes snippet for cluster mempool"

Writing nit: `eg` -> `e.g.,`.

💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573692555)
In commit " Warn user if using -limitancestorsize/-limitdescendantsize that the options have no effect"

Perhaps mention the relevant corresponding option names (`-limitclustercount` and `-limitclustersize`).

📝 billymcbip opened a pull request: "test: Add DERSIG unit tests to script_tests.json"
(https://github.com/bitcoin/bitcoin/pull/33973)
I'd like to add a few `DERSIG` variants of existing `STRICTENC` tests, since `DERSIG` is a consensus flag.

I ran:
```
cmake -B build -DENABLE_WALLET=OFF
cmake --build build --parallel 8
ctest --test-dir build --parallel 8
```
```
6/133 Test #93: script_tests ......................... Passed 1.57 sec
```

💬 JeremyRubin commented on pull request "[CI] remove `doc/release-notes.md` from lint-spelling.py":
(https://github.com/bitcoin/bitcoin/pull/33968#issuecomment-3592667408)
I think it's a separate issue -- since it seems that the release (e.g., non code doc changes pre-release) is maybe not getting run through CI anyways, or else this would have been caught?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3592671116)
Thank you very much for your review and bencharking @l0rinc! The speedup this offers is great. I have taken most of your suggestions.

📝 hebasto opened a pull request: "cmake: Check dependencies after build option interaction"
(https://github.com/bitcoin/bitcoin/pull/33974)
At present, `CMakeLists.txt` interleaves configuration-option handling with dependency discovery. As a result, unnecessary checks may be performed. For example:
```
$ cmake -B build --preset dev-mode -DBUILD_FOR_FUZZING=ON
<snip>
-- Found PkgConfig: /usr/bin/pkg-config (found version "2.3.0")
-- Found ZeroMQ: /usr/lib64 (found suitable version "4.3.5", minimum required is "4.0.0")
-- Performing Test HAVE_USDT_H
-- Performing Test HAVE_USDT_H - Success
-- Found USDT: /usr/include
-- Foun
...

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2574049689)
I now use a leveldb and add mock input data before the benchmark, so it's more of a real world benchmark now :+1: . Thanks!

💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2574105954)
Removed.

💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2574107800)
Removed.

💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2574108138)
Done.

💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2574112696)
Added a reference to `-limitclustersize` since it's analogous.

💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2574114517)
Fixed.

💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2574115280)
Yep, that is what I meant! Fixed.

💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2574119436)
Agreed, "performance" is misleading. I was thinking "better behavior" myself, but took your language.

💬 sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2574120039)
Done.