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

👍 willcl-ark approved a pull request: "guix: disable libsanitizer in Linux GCC build"
(https://github.com/bitcoin/bitcoin/pull/33780#pullrequestreview-3420936380)
utACK 5c41fa2918c8fee36d0e0375e753249f1efa7c07

💬 fanquake commented on pull request "Modernize custom filtering":
(https://github.com/bitcoin-core/gui/pull/899#issuecomment-3490255856)
Backported to `30.x` in https://github.com/bitcoin/bitcoin/pull/33609

🚀 fanquake merged a pull request: "guix: disable libsanitizer in Linux GCC build"
(https://github.com/bitcoin/bitcoin/pull/33780)

👍 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