👍 rkrux approved a pull request: "test: remove obsolete `get_{key,multisig}` helpers from wallet_util.py"
(https://github.com/bitcoin/bitcoin/pull/33782#pullrequestreview-3420985296)
crACK ec8516ceb7568d7b09836b830023978bd37f8462

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2493783865)
I had a f2f discussion with somebody (forgot who?) of what happens if a transaction is submitted for private broadcast and is:

1. previously submitted but not yet in the mempool
2. in the mempool (and thus not in the for-private-broadcast list)

This part of the test answers 1. - if a transaction is already in the internal for-private-broadcast list then `sendrawtransaction` will error with "Ignoring unnecessary request...".

"2." is not exercised in the test, but what will happen is tha
...

🚀 fanquake merged a pull request: "clang-tidy: Remove no longer needed NOLINT"
(https://github.com/bitcoin/bitcoin/pull/33781)

💬 fanquake commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#discussion_r2493801897)
> and the maybe-uninitialized below can be removed?

Fixed the URL, I don't think we can drop it yet: https://github.com/bitcoin/bitcoin/actions/runs/19074211079/job/54485090675?pr=33775#step:8:3723.

💬 hebasto commented on pull request "cmake: Move IPC tests to `ipc/test`":
(https://github.com/bitcoin/bitcoin/pull/33774#issuecomment-3490333396)
> should `src/test/kernel` also be moved to `src/kernel/test` to keep the project structure Consistent. ?

Maybe.

Moving IPC tests provides additional benefits. I'm not sure the same justification applies to the kernel tests, so let's leave them for a follow-up.

👍 TheCharlatan approved a pull request: "test: Add bitcoin-chainstate test for assumeutxo functionality"
(https://github.com/bitcoin/bitcoin/pull/33728#pullrequestreview-3421048286)
re-ACK 67740dfe6cfe3f3f91aaad41018a7da9004dcd8c

👍 willcl-ark approved a pull request: "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors"
(https://github.com/bitcoin/bitcoin/pull/33779#pullrequestreview-3421197426)
ACK a8a33bc0c0a11093418debc36db8ac63bf90e687

Indeed a good time to get this change in :)

💬 maflcko commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3490481643)
lgtm ACK 25ede968badadfeeaaa8358b38c9a8cfff7189f0

💬 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!