✅ marcofleon closed a pull request: "refactor: ensure type safety for txid and wtxid in `RelayTransaction`"
(https://github.com/bitcoin/bitcoin/pull/31001)

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948876683)
I am considering this, but just realized that the distinction is not "proxy vs no-proxy" because in both cases of the `std::variant` we may end up connecting via the proxy. The logic (as convoluted as it is, it is the same in `master`) is:

* if connecting to `CService`
* if to an I2P `CService` then proxy must be used
* otherwise the proxy is optional, if provided it will be used
* if connecting to string host, then proxy must be used

So, if `std::variant` is to be avoided and two f
...

💬 hebasto commented on pull request "build: set build type and per-build-type flags as early as possible":
(https://github.com/bitcoin/bitcoin/pull/31711#issuecomment-2647711577)
I'd like to summarize here a few observations regarding CMake behaviour.

1. Single-config generators, such as "Ninja":
- `CMAKE_BUILD_TYPE` is defined in `enable_language()`.
- Compiler checks in `enable_language()` ignore any configuration, unless `CMAKE_TRY_COMPILE_CONFIGURATION` is set beforehand.

2. Single-config generators, such as "Ninja Multi-Config":
- `CMAKE_CONFIGURATION_TYPES` is defined in `project()`.
- Compiler checks in `enable_language()` use the "Debug" configuration (
...

👍 stickies-v approved a pull request: "cmake: Install man pages for configured targets only"
(https://github.com/bitcoin/bitcoin/pull/31765#pullrequestreview-2605507226)
tACK c783833f7c74bb184a74f3813b0bb973ea0b9e11

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948908109)
Done.

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2647781404)
`7d84f431f9...7b63b4ca1c`: minor repush to elaborate a commit message as suggested.

Are you in the mood of reviewing a change to this PR, https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2485715952, that stores the "client" objects in `SockMan` (as `shared_ptr`, not as raw pointers)? In the case of `CConnman` that is `CNode`. This will:
* make the interaction between `SockMan` and `CConnman` simpler
* probably faster (less lookups by id)
* resolve the issue in https://github.com/
...

👍 hebasto approved a pull request: "build: set build type and per-build-type flags as early as possible"
(https://github.com/bitcoin/bitcoin/pull/31711#pullrequestreview-2605560113)
ACK 85c6bafe0b93ca180df3e0ec5c3f1c8d1fefbf9f, tested on Ubuntu 24.10 with the "Ninja" and "Ninja Multi-Config" generators.

A single check still uses the default flags:https://github.com/bitcoin/bitcoin/blob/6a46be75c43e6489d7cb2aaea614ba552b31b35a/cmake/module/TryAppendCXXFlags.cmake#L125

This issue can be addressed with the following patch:
```diff
--- a/cmake/module/ProcessConfigurations.cmake
+++ b/cmake/module/ProcessConfigurations.cmake
@@ -4,8 +4,6 @@

include_guard(GLOBAL)

...

👍 hebasto approved a pull request: "cmake: add optional source files to bitcoin_crypto and crc32c directly"
(https://github.com/bitcoin/bitcoin/pull/31268#pullrequestreview-2605586589)
re-ACK 9cf746d6631739df9c9f80accd5812b319efcfec.

💬 hodlinator commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2647888735)
Would really like to get rid of the custom ref-counting of `CNode` and avoid id-lookups. But fear it will grow this PR a bit much.

Why not remove the custom ref-counting as a separate PR to begin with? Things like https://github.com/bitcoin/bitcoin/pull/28222/commits/af78c48ff82190e000501b14470cca32811f02a2#r1288400379 from #28222 make me think it may not be trivial, but maybe you have a more elegant approach.

I personally commit to review regardless of whether you split it out to it's own
...

👍 hebasto approved a pull request: "guix: remove test-security/symbol-check scripts"
(https://github.com/bitcoin/bitcoin/pull/31818#pullrequestreview-2605659512)
ACK 76c090145e9bb64fe4ef6a663723dd0e9028ed10.

My Guix build:
```
aarch64
4756d1c55efefe2dd3429d93060555617bb00709772a3c13638aab8c8dd9af2c guix-build-76c090145e9b/output/aarch64-linux-gnu/SHA256SUMS.part
f9fe3c2cb751060366eae0070a1c91897ac49c74bfc53baafca65cb17413ce7e guix-build-76c090145e9b/output/aarch64-linux-gnu/bitcoin-76c090145e9b-aarch64-linux-gnu-debug.tar.gz
6532576a94838e11f8aa98b037f116af32b71eb6f446f67ffc451207b442fd36 guix-build-76c090145e9b/output/aarch64-linux-gnu/bitcoi
...

💬 maflcko commented on issue "CI: Failed pulls from docker.io causing jobs to fail":
(https://github.com/bitcoin/bitcoin/issues/31797#issuecomment-2647933720)
If someone set's up a new CI machine, one could consider the local build cache from 6835e9686c41b31d14ef25dc8df9409a99f079a8 as a fix, but I haven't tried if that works with podman.

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-2647951114)
Rebased

💬 glozow commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1949032017)
Changed to `int64_t` in #31829

💬 glozow commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#issuecomment-2647952256)
Next PR is #31829

💬 hebasto commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1949049901)
Thanks! Applied.

✅ glozow closed an issue: "Duplicate coinbase transaction space reservation in CreateNewBlock"
(https://github.com/bitcoin/bitcoin/issues/21950)

🚀 glozow merged a pull request: "mining: bugfix: Fix duplicate coinbase tx weight reservation"
(https://github.com/bitcoin/bitcoin/pull/31384)

👋 glozow's pull request is ready for review: "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs"
(https://github.com/bitcoin/bitcoin/pull/31829)

📝 Sjors opened a pull request: "build: disable bitcoin-node if daemon is not built"
(https://github.com/bitcoin/bitcoin/pull/31834)
When building for fuzzing with multiprocess enabled, we were still trying to build `bitcoin-node`. This PR fixes that, by applying a similar check as for `bitcoin-gui`.

Before:

```
cmake -B build -DBUILD_FOR_FUZZING=ON -DWITH_MULTIPROCESS=ON

...

Configure summary
=================
Executables:
bitcoind ............................ OFF
bitcoin-node (multiprocess) ......... ON
bitcoin-qt (GUI) .................... OFF
bitcoin-gui (GUI, multiprocess) ..... OFF
```

Aft
...

💬 hebasto commented on pull request "build: disable bitcoin-node if daemon is not built":
(https://github.com/bitcoin/bitcoin/pull/31834#issuecomment-2647999846)
> When building for fuzzing with multiprocess enabled, we were still trying to build `bitcoin-node`. This PR fixes that...

But 7e11ee6ad9b04546b900e38dee349db821415ba7 modifies only the summary output.