💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2334213839)
Confirmed that at least for example 19, it simply can't find the optimal solution until the commit it's included, even if total iterations are cranked up a couple orders of magnitude.

💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2334217170)
Example cluster 18:

💬 fanquake commented on pull request "cmake: add `USE_SOURCE_PERMISSIONS` to all `configure_file()` usage":
(https://github.com/bitcoin/bitcoin/pull/30823#issuecomment-2334225403)
Guix Build:
```bash
f2e883f15be3d30cc27f1e5ba96ccaa18a9201a242f6dfa7bc65595192249f47 guix-build-1f054eca4e77/output/arm64-apple-darwin/SHA256SUMS.part
53379e0ac143a7c150563ac10973ee7af5dc827e53f96a8b2519cdf1992ece63 guix-build-1f054eca4e77/output/arm64-apple-darwin/bitcoin-1f054eca4e77-arm64-apple-darwin-unsigned.tar.gz
8446ce177d54bac5b185faaecf7a7f34590b512b8ffc690f43d461418196f99c guix-build-1f054eca4e77/output/arm64-apple-darwin/bitcoin-1f054eca4e77-arm64-apple-darwin-unsigned.zip
88
...

💬 zaidmstrr commented on pull request "fix: handle invalid `-rpcbind` port earlier":
(https://github.com/bitcoin/bitcoin/pull/30679#issuecomment-2334241361)
Hey, I'm reviewing this PR, and I saw that the test `run_invalid_bind_test` you implemented in `rpc_bind.py` is not invoking without explicitly providing the `--ipv4` or `--ipv6` flags, so is there any particular reason for that?

📝 hebasto opened a pull request: "build: Introduce "Kernel" installation component"
(https://github.com/bitcoin/bitcoin/pull/30835)
This PR enables building and installing only `libbitcoinkernel`, without the need to disable other targets during the project build system generation:

```
$ rm -rf build && cmake -B build -DBUILD_KERNEL_LIB=ON
$ cmake --build build --target bitcoinkernel
$ cmake --install build --component Kernel --prefix /home/hebasto/INSTALL
-- Install configuration: "RelWithDebInfo"
-- Installing: /home/hebasto/INSTALL/lib/libbitcoinkernel.so
```

Please note, that only the `bitcoinkernel` target i
...

💬 ryanofsky commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2334269708)
Rebased e225f7cbc3e6842f8e7f1c482c2aacd810e99c1b -> 30073e6b3a24cbe417c45cd5df6a3a2de0251e9d ([`pr/ipc-bind.6`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-bind.6) -> [`pr/ipc-bind.7`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-bind.7), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-bind.6-rebase..pr/ipc-bind.7)) with no changes other than updating system.cpp fuzz test

👍 fanquake approved a pull request: "test: Work around boost compilation error"
(https://github.com/bitcoin/bitcoin/pull/30834#pullrequestreview-2286528733)
ACK fa9d7d5d205ada8915cbbc29599ab8e7bf1fffe0

🚀 fanquake merged a pull request: "test: Work around boost compilation error"
(https://github.com/bitcoin/bitcoin/pull/30834)

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2334282254)
Rebased c538ec69f266b51c893a374a4bb82796ede3d7cb -> 083b886bc6b98fab53e2c6b7dc99a017daa9726e ([`pr/mine-types.4`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.4) -> [`pr/mine-types.5`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.5), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine-types.4-rebase..pr/mine-types.5)) updating for cmake

💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2334289694)
Rebased ec5f45b9186184cac9c64ed08b8b8cc596e301a3 -> 697ba11c9496e3a2aaf6fc3ba64fd39a028109c2 ([`pr/mine.8`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.8) -> [`pr/mine.9`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.9), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.8-rebase..pr/mine.9)) just updating for cmake

👍 l0rinc approved a pull request: "util: Use consteval checked format string in FatalErrorf"
(https://github.com/bitcoin/bitcoin/pull/30546#pullrequestreview-2286549302)
ACK fa18f7cc9ac36ebe0fa8c381b1db76085812e95f

💬 l0rinc commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1747300351)
can we reduce the duplication between line 39 and this set of values?

💬 achow101 commented on pull request "test: Work around boost compilation error":
(https://github.com/bitcoin/bitcoin/pull/30834#issuecomment-2334311818)
Backported fa9d7d5d205ada8915cbbc29599ab8e7bf1fffe0 in #30827

💬 achow101 commented on pull request "[28.x] Further backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/30827#discussion_r1747314219)
Backported fa9d7d5d205ada8915cbbc29599ab8e7bf1fffe0

💬 l0rinc commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`, too":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1747327065)
I have [added](https://github.com/bitcoin/bitcoin/compare/ab875e14b7ed02e6e4bdc607d67e28bc10216a27..14080bb53d649ed1c99f121f377d2c8f2019c99e) an `std::byte` overload in a separate commit, so now we have:
```C++
CScript& operator<<(const std::span<const std::byte> b) LIFETIMEBOUND
{
return *this << std::bit_cast<std::span<uint8_t>>(b);
}
```
which delegates to:
```C++
CScript& operator<<(const std::span<const unsigned char> b) LIFETIMEBOUND
{
AppendDataSize(b.size());
Appe
...

💬 hebasto commented on pull request "kernel: Create usable static kernel library":
(https://github.com/bitcoin/bitcoin/pull/30814#issuecomment-2334333003)
766881e33d13ca2887f7ed179cdcc6bc007a6325

While installing `libbitcoinkernel.pc`, why ignore CMake's configs for downstream projects using CMake?

💬 maflcko commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`, too":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1747339084)
Probably fine here, but IIRC it is UB to cast away const from something that is always const.

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1747342326)
Yes, but it will be more code and complexity, so I'll leave this as-is for now.

💬 sipa commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`, too":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1747344345)
I believe casting away constness on itself is never UB, but *modifying* a const value (through a pointer/reference whose constness was cast away) is UB.

🤔 tdb3 reviewed a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2286432258)
Approach ACK

Thanks for the useful RPC. Love that for some use cases it enables the option to avoid ancillary software (e.g. electrum server).

Left some initial comments for now (mainly for tests). Planning to circle back.