💬 jonatack commented on pull request "doc: fix 3 simple CI codespell warnings":
(https://github.com/bitcoin/bitcoin/pull/30700#discussion_r1730131890)
We already do the same for words in the ignore list like invokable, requestor, unparseable, useable, and warmup. (It looks like "warmup" can be removed from the ignore list now.)

💬 jonatack commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1730137103)
I think the new test file needs to be added to the runner.

```diff
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -396,6 +396,7 @@ BASE_SCRIPTS = [
'feature_config_args.py',
'feature_presegwit_node_upgrade.py',
'feature_settings.py',
+ 'rpc_getdescriptoractivity.py',
'rpc_getdescriptorinfo.py',
'rpc_mempool_info.py',
'rpc_help.py',
```

💬 theStack commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#discussion_r1730163427)
nit: imho slightly preferred, for not having to repeat the type
```suggestion
if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, (sockopt_arg_type)&one, sizeof(one)) == SOCKET_ERROR) {
```

👍 theStack approved a pull request: "http: set TCP_NODELAY when creating HTTP server"
(https://github.com/bitcoin/bitcoin/pull/30675#pullrequestreview-2259013554)
Tested ACK c1bec8269712059705d5f416257ad5201ec3d4a8

With the benchmark instructions from the opening post (using Apache's `ab` tool, found in the `apache2-utils` package on Debian Linux) adapted to fetch a recent block, this leads to a >10x speed-up on my machine: mean `Time per request` is 46.617ms on master vs. 3.717ms on the PR.

👍 theStack approved a pull request: "test: XORed blocks test follow up"
(https://github.com/bitcoin/bitcoin/pull/30669#pullrequestreview-2259021660)
ACK ab9d2451c47fd30abecaea3f8d8a6e33541911d7

💬 theStack commented on pull request "test: XORed blocks test follow up":
(https://github.com/bitcoin/bitcoin/pull/30669#discussion_r1730169778)
an 's' got lost on the move up
```suggestion
# from InitBlocksdirXorKey::xor_key.size()
```

💬 tdb3 commented on pull request "test: XORed blocks test follow up":
(https://github.com/bitcoin/bitcoin/pull/30669#discussion_r1730172019)
Thanks. Fixed.

👍 theStack approved a pull request: "test: XORed blocks test follow up"
(https://github.com/bitcoin/bitcoin/pull/30669#pullrequestreview-2259025660)
re-ACK 1c403af92be13cbdfa6ab1906f8cc34fabc21e63

👍 tdb3 approved a pull request: "test: Avoid intermittent block download timeout in p2p_ibd_stalling"
(https://github.com/bitcoin/bitcoin/pull/30705#pullrequestreview-2259034843)
CR ACK fa5b58ea01fac1adb6336b8b6b5217193295c695

💬 romanz commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#discussion_r1730252361)
Thanks! Fixed in 03d49d0f25.

👍 theStack approved a pull request: "http: set TCP_NODELAY when creating HTTP server"
(https://github.com/bitcoin/bitcoin/pull/30675#pullrequestreview-2259196575)
re-ACK 03d49d0f25ab5660524d5ddd171de677a808b984

💬 l0rinc commented on pull request "doc: fix 3 simple CI codespell warnings":
(https://github.com/bitcoin/bitcoin/pull/30700#discussion_r1730299457)
Not sure, in other cases we've just fixed "re-used": https://github.com/bitcoin/bitcoin/pull/28605/files#diff-794891fcb80950b0c81f330c416efdf5d35789fb56a91a18dcc25266d7cdccfdL55

💬 l0rinc commented on pull request "doc: fix 3 simple CI codespell warnings":
(https://github.com/bitcoin/bitcoin/pull/30700#discussion_r1730300095)
Lol, seems like everyone is talking about this: https://github.com/codespell-project/codespell/issues/3521

💬 maflcko commented on pull request "test: XORed blocks test follow up":
(https://github.com/bitcoin/bitcoin/pull/30669#discussion_r1730300469)
style nit, while touching this line: The argument is of type Path, so I think you can just call `unlink()` on it to avoid the `os` import?

💬 l0rinc commented on pull request "doc: fix 3 simple CI codespell warnings":
(https://github.com/bitcoin/bitcoin/pull/30700#issuecomment-2308765191)
> Probably preferable to avoid needless warnings

Since we seem to ignore many other QT related files and folder, I've added this one to the excludes - what do you think?

🤔 fjahr reviewed a pull request: "devtools, utxo-snapshot: Fix block height out of range in script"
(https://github.com/bitcoin/bitcoin/pull/30690#pullrequestreview-2259216416)
tACK 5b4f34006dbd76223b55b156421f87d2241ac296

Reviewed the code and did some light testing.

It's unclear when #29553 will get merged and since we may see some people come in and try out assumeutxo for the first time when v28 is released so I think it's ok to keep improving the script until then.

This could have also been fixed in a simpler way by moving the `PIVOT_BLOCKHASH=` line below the two `trap` lines but I guess it's nice to give users an understandable error message so I am fine
...

💬 fjahr commented on pull request "devtools, utxo-snapshot: Fix block height out of range in script":
(https://github.com/bitcoin/bitcoin/pull/30690#discussion_r1730310284)
nit: Could reuse that below in the `PIVOT_BLOCKHASH=` line but only if you have to retouch.

💬 l0rinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2308803683)
> Comparing the `objdump` of `sigopcount_tests.cpp.o` and `validation_chainstate_tests.cpp.o` revealed that for some reason ccache returned the content of `sigopcount_tests` for `validation_chainstate_tests` (exact match, 0 difference between the two).

I investigated further, and if I understand correctly, it appears that `ccache` is indeed returning `sigopcount_tests` binaries for `validation_chainstate_tests`.

I generated build debug logs using the following commands:
```bash
export CC
...

📝 maflcko opened a pull request: " fuzz: Add missing fuzz targets to cmake build "
(https://github.com/bitcoin/bitcoin/pull/30712)
Fixes https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1726881676

Can be tested via:

```
PRINT_ALL_FUZZ_TARGETS_AND_ABORT=1 ./bld-autot/src/test/fuzz/fuzz > /tmp/f_autot
PRINT_ALL_FUZZ_TARGETS_AND_ABORT=1 ./bld-cmake/src/test/fuzz/fuzz > /tmp/f_cmake
diff --unified /tmp/{f_autot,f_cmake}

💬 tdb3 commented on pull request "test: XORed blocks test follow up":
(https://github.com/bitcoin/bitcoin/pull/30669#discussion_r1730330695)
Thanks, that's cleaner. Added.