💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2659622448)
Rebased edb8a72226b362ee60e4184fd8f56d1e588ce5b6 -> 27bb348388a6708e8748d7073e352c047e8fb95c ([`pr/wrap.19`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.19) -> [`pr/wrap.20`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.20), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/wrap.19-rebase..pr/wrap.20)) due to silent conflict with #31844

📝 fanquake opened a pull request: "doc: add missing copyright headers"
(https://github.com/bitcoin/bitcoin/pull/31864)
Add & amends a number of copyright headers.
Remove some now obselete doc/code.

📝 fanquake opened a pull request: "build: move `rpc/external_signer` to node library"
(https://github.com/bitcoin/bitcoin/pull/31865)
Move `rpc/external_signer` from `bitcoin_common` to `bitcoin_node`.
Remove the check-deps suppression.

📝 ryanofsky opened a pull request: "test, refactor: Add TestNode.binaries to hold binary paths"
(https://github.com/bitcoin/bitcoin/pull/31866)
Add new `TestNode.binaries` object to manage paths to bitcoin binaries.

The `binaries` object makes it possible for the test framework to exercise the bitcoin wrapper executable introduced in https://github.com/bitcoin/bitcoin/pull/31375 and also makes it easier in general to add new binaries, and new options and environment variables controlling how they are invoked, because logic for invoking them that was previously spread out is now consolidated in one place.

These changes were origina
...

💬 maflcko commented on pull request "refactor: Remove redundant definitions":
(https://github.com/bitcoin/bitcoin/pull/29492#issuecomment-2659695808)
tend towards NACK, without a motivation (example of a real bug that this prevents or prevented):

* Given a motivation, it should be enforced with clang-tidy (or otherwise)
* Without motivation, it shouldn't be enforced, nor should the changes be made in the first place

💬 luisfg30 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1956383771)
Unused variable `res`

```suggestion
_, wallet = self.migrate_and_get_rpc("taproot")
```

💬 maflcko commented on pull request "build: move `rpc/external_signer` to node library":
(https://github.com/bitcoin/bitcoin/pull/31865#issuecomment-2659736131)
lgtm ACK e501246e77cc36cff222fb07aed2fd1316e11e19

💬 s373nZ commented on pull request "cmake: add optional source files to bitcoin_crypto and crc32c directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2659736519)
ACK 9cf746d6631739df9c9f80accd5812b319efcfec

Tested compilation on Ubuntu 24.04 x86_64. Learned a lot looking over this PR. Nice work!

💬 hebasto commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2659741940)
Tested a Windows-specific part:
```
$ env HOSTS=x86_64-w64-mingw32
$ ./contrib/guix/guix-build
$ ./contrib/guix/guix-codesign
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
ac50a82cb146e016d5d643460dc4ff7452a70497f2d95f76cee2bcfd82724ab6 guix-build-096525e92cc2/output/dist-archive/bitcoin-096525e92cc2-codesignatures-e252afe1296a.tar.gz
504608f78dc2be04bda41dc212d3cbb09afd270485884b03b426ad596b4b3611 guix-build-0965
...

💬 fanquake commented on pull request "cmake: add optional source files to bitcoin_crypto and crc32c directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2659742890)
@theuni could you circle back here?

💬 hebasto commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#discussion_r1956420649)
> @hebasto Looking at this again, is there any reason [this needs to be an object library](https://github.com/bitcoin/bitcoin/blob/master/cmake/minisketch.cmake#L43)?

I don't thin so.

> It triggers the bug that necessitates the workaround here. I'm wondering if rather than using our hack or the (better) workaround suggested upstream, if we should just make it an archive library and avoid the issue altogether?

From [Professional CMake: A Practical Guide 20th Edition](https://discourse.cm
...

👍 TheCharlatan approved a pull request: "build: move `rpc/external_signer` to node library"
(https://github.com/bitcoin/bitcoin/pull/31865#pullrequestreview-2618309064)
ACK e501246e77cc36cff222fb07aed2fd1316e11e19

💬 ajtowns commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2659802629)
ACK 4080b66cbec2b6fc2fcfd7356941236f65d508e3 - light review

Did you know 48% of utxos are under 1000 sats ($1)? With this PR you can find that out, and also how recent those utxos are:

```
sqlite> select count(*) from utxos;
178306838
sqlite> select count(*)/1783068 from utxos where value < 1000;
48
sqlite> select count(*)/1783068, (height/50000)*50000 from utxos where value < 1000 group by height/50000;
0|50000
0|100000
0|150000
0|200000
0|250000
0|300000
0|350000
0|400000
0
...

📝 Syed-AbdurRahaman2006 opened a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/31867)
Line 6:
Before: For an immediately usable, binary version of the Bitcoin Core software, see https://bitcoincore.org/en/download/. After: For an immediately usable binary version of the Bitcoin Core software, visit [Bitcoin Core Downloads](https://bitcoincore.org/en/download/). Removed the comma after "usable" for better flow, replaced "see" with "visit" for action-oriented language, and turned the URL into a clickable link. Line 16:
Before: Further information about Bitcoin Core is available i
...

📝 l0rinc opened a pull request: "optimization: speed up block serialization"
(https://github.com/bitcoin/bitcoin/pull/31868)
This PR contain a few different optimization I found by profiling IBD and the newly added block seralization benchmarks.

The commits merge similar (de)serialization methods and separates them internally with `if constexpr` - similarly to how it has been [done here before](https://github.com/bitcoin/bitcoin/pull/28203). This enabled further `SizeComputer` otimizations as well.

Other than these, since single byte writes are used very often (used for every `(u)int8_t` or `std::byte` or `bool
...

💬 theuni commented on pull request "cmake: add a component for each binary":
(https://github.com/bitcoin/bitcoin/pull/31844#issuecomment-2659817050)
> > @stickies-v Thanks for pointing that out, that was accidental! I actually didn't notice the difference. I think we should fix it so that they're aligned instead of having a single outlier.
> > ~Though.. would it make sense to rename the target to `libbitcoinkernel` instead of changing the component to `bitcoinkernel`? I think the former would be more obvious?~
> > Edit: nm, can't do that as CMake will rename the resulting file to liblibbitcoinkernel.so.
>
> But we can:
>
> ```cmake

...

💬 l0rinc commented on pull request "doc: Fix description of byte order of hashes in ZMQ documentation":
(https://github.com/bitcoin/bitcoin/pull/31862#issuecomment-2659824080)
Concept ACK

💬 l0rinc commented on pull request "Update README.md":
(https://github.com/bitcoin/bitcoin/pull/31867#issuecomment-2659840197)
NACK - opinionated random changes

✅ fanquake closed an issue: "CMake `CheckPIESupported` doesn't always work"
(https://github.com/bitcoin/bitcoin/issues/30771)

🚀 fanquake merged a pull request: "cmake: Add `CheckLinkerSupportsPIE` module"
(https://github.com/bitcoin/bitcoin/pull/31359)