💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2493940857)
> In this implementation the reserved memory is kept in between blocks

K, so let's reserve outside and clear inside.
Now that missing values aren't failures, we can experiment with shortids - since a missing value isn't a failure anymore (even though I wouldn't expect any collisions in 64 bits either, assuming uniform distribution. But even if the distribution isn't uniform, we can likely store it safely).
64 bit ids for internal spends would mean that in case of some collision we will atte
...

👋 fanquake's pull request is ready for review: "guix: use GCC 14.3.0 over 13.3.0"
(https://github.com/bitcoin/bitcoin/pull/33775)

👍 stickies-v approved a pull request: "doc: add cmake help option in Windows build docs"
(https://github.com/bitcoin/bitcoin/pull/33789#pullrequestreview-3421363751)
ACK 9577daa3b88a8fbe8c96d2848adcc88c55c55a29

👍 rkrux approved a pull request: "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script"
(https://github.com/bitcoin/bitcoin/pull/31560#pullrequestreview-3421406013)
lgtm ACK ed02f67c5831d94720e55127216aa0d97f55e72a

💬 rkrux commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r2494075905)
In ed02f67c5831d94720e55127216aa0d97f55e72a "contrib: add script dump_to_sqlite.sh for direct SQLite3 UTXO dump"

Nit: Can call the script `dump_utxo_to_sqlite.sh` for clarity even though it resides in `utxo-tools/`.

👍 hebasto approved a pull request: "doc: add cmake help option in Windows build docs"
(https://github.com/bitcoin/bitcoin/pull/33789#pullrequestreview-3421455626)
ACK 9577daa3b88a8fbe8c96d2848adcc88c55c55a29.

💬 naiyoma commented on issue "importdescriptors: check for errors before rescanning":
(https://github.com/bitcoin/bitcoin/issues/33655#issuecomment-3490788538)
Currently, if there are any validation errors, they're added in the result and returned after a rescan . I think we should validate all descriptors first without modifying the wallet. If any descriptor fails validation, throw an error immediately and abort— no rescan.
A possible fix for this:

```diff
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index 4b4e43
...

💬 instagibbs commented on pull request "refactor: remove dead branches in `SingletonClusterImpl`":
(https://github.com/bitcoin/bitcoin/pull/33768#issuecomment-3490803624)
ACK 2d23820ee11678d567c75f94c40011ed9f0e274f

👍 stickies-v approved a pull request: "script: remove dead code in `CountWitnessSigOps`"
(https://github.com/bitcoin/bitcoin/pull/33786#pullrequestreview-3421542558)
ACK 24bcad3d4df59690f30c9df8ebb62f0bddd0f1c7

📝 Ahfaz001 opened a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/33790)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 l0rinc commented on pull request "log: reduce excessive "rolling back/forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2494434316)
Interesting, though I'm not sure what value hundreds of thousands of logs saying basically the same thing provide.
Do you mind if we tackle this in a follow-up instead?

👍 ryanofsky approved a pull request: "cmake: Move IPC tests to `ipc/test`"
(https://github.com/bitcoin/bitcoin/pull/33774#pullrequestreview-3421970095)
Code review ACK f26c8b03f968843e6e1381d1e1721f4379f7e7ab. Nice change avoiding the need to duplicate clang-tidy suppressions. Thanks for the followup!

💬 ryanofsky commented on pull request "cmake: Move IPC tests to `ipc/test`":
(https://github.com/bitcoin/bitcoin/pull/33774#discussion_r2494445665)
In commit "cmake, test: Improve locality of `bitcoin_ipc_test` library description" (f26c8b03f968843e6e1381d1e1721f4379f7e7ab)

I think this comment is still useful to keep because otherwise there is no explanation of why `bitcoin_ipc_test` in defined `src/ipc/` instead of `src/ipc/test` where it would make more sense.

Comment would just need to be tweaked slightly to be kept: `src/CMakeLists.txt` -> `src/ipc/CMakeLists.txt`, `src/test/CMakeLists.txt` -> `src/ipc/test/CMakeLists.txt`, and `
...

💬 yancyribbens commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3491224946)
Forgive my interjection, although isn't there practice in place to routinely audit DNS seed needs? IE a chron job that checks what versions are being named and the validity of the content? If such an audit system is in place, run on publicly verifiable infrastructure, then there's indisputable reason to remove a node that names invalid content. Also, if there is a high bar for auditing seed DNS seed nodes, I see no reason why more people can't run these nodes, not less.

Also, it would be rea
...

💬 waketraindev commented on pull request "doc: add cmake help option in Windows build docs":
(https://github.com/bitcoin/bitcoin/pull/33789#issuecomment-3491311987)
ACK 9577daa

💬 maflcko commented on pull request "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors":
(https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3491314532)
review ACK a8a33bc0c0a11093418debc36db8ac63bf90e687 🐮

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK a8a33bc0c0a1
...

💬 TheCharlatan commented on pull request "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors":
(https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3491343671)
I tried to reproduce what the CI is doing locally. Having to run the full CI either on push or locally just to fix the includes is not ideal. The following command seems to produce different results:

```
/home/user/Downloads/include-what-you-use/iwyu_tool.py -p /home/user/bitcoin/build_dev_mode_clang/compile_commands.json src/kernel/chainparams.cpp -- -Xiwyu --cxx17ns -Xiwyu --mapping_file=/home/user/bitcoin/contrib/devtools/iwyu/bitcoin.core.imp -Xiwyu --max_line_length=160

(/home/user
...

💬 ryanofsky commented on pull request "init: Require explicit -asmap filename":
(https://github.com/bitcoin/bitcoin/pull/33770#issuecomment-3491348202)
Thanks for the suggestions and reviews! Implemented some test cleanup below.

I do want to wait for feedback from @fjahr before going ahead with this PR since he expressed a preference for keeping behavior unchanged until #28792 is merged ("I am again leaning towards keeping the current behavior for the intermediary step" from https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3475259644) and I think would that would be reasonable. But I do think this PR is an improvement and resolutio
...

💬 ryanofsky commented on pull request "init: Require explicit -asmap filename":
(https://github.com/bitcoin/bitcoin/pull/33770#discussion_r2494531445)
re: https://github.com/bitcoin/bitcoin/pull/33770#discussion_r2490759547

> [cfeb160](https://github.com/bitcoin/bitcoin/commit/cfeb160baec1369452c42d05c51f2a6af76ed077): nit: Perhaps we could remove this `(default: none)` to maintain consistency with other `=<file>` options?

Thanks for the suggestion. I looked into it and initially it seemed like a good simplification, but then I noticed that this is not the only `(default: none)` mention in help since there is also `-i2psam`. Also since P
...

💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#issuecomment-3491367389)
Rebased 6137cecfd3a448c2befe043849fce0b4ea2cd1fe -> 53abfae26ffbdd09daff66fd253171bbab1b550e ([chainman_flush_destructor_7](https://github.com/TheCharlatan/bitcoin/tree/chainman_flush_destructor_7) -> [chainman_flush_destructor_8](https://github.com/TheCharlatan/bitcoin/tree/chainman_flush_destructor_8), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainman_flush_destructor_7..chainman_flush_destructor_8))

* Fixed conflict with #30595 - the PR now simplifies the bindings glue cod
...