💬 vasild commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1791761963)
```suggestion
LogDebug(BCLog::NET,
"socket receive timeout: %is, %s\n", count_seconds(now - last_recv),
node.DisconnectMsg(fLogIPs)
```

💬 vasild commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1791784114)
This log shouldn't contain "disconnecting" because we do not disconnect.

💬 vasild commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1791764812)
This `%d %d`, `last_recv.count() != 0, last_send.count() != 0` is super obscure. It was like that before, so it is fine wrt this PR.

But would be nice to fix it, since this is changing the same log message already. I am not sure how though. Maybe change that `%d` to something like "has never received from the peer" + "has never sent to the peer".

⚠️ dergoegge opened an issue: "Disallow building fuzz binary without `-DBUILD_FOR_FUZZING`"
(https://github.com/bitcoin/bitcoin/issues/31057)
Without `-DBUILD_FOR_FUZZING` the fuzz binary is less suited for testing puporses, because it won't crash on `Assume` and it won't work around fuzz blockers with `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION`. Afaict, the only place where the fuzz binary is currently used without `-DBUILD_FOR_FUZZING` is in the macOS and windows CI jobs.

I would like to propose that we split the CI jobs into separate fuzz-only jobs that use `-DBUILD_FOR_FUZZING` and that we remove support for building the fuzz bi
...

💬 dergoegge commented on issue "Disallow building fuzz binary without `-DBUILD_FOR_FUZZING`":
(https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2399758280)
cc @maflcko @marcofleon

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2399781582)
> > Ninja is required to build the `qt` package in depends. It is mentioned in `depends/README.md`. However, `doc/build-osx.md` does not cover building with depends, so I don't think it is necessary to mention Ninja there.
>
> Could you please explain why this is the case? Ninja seems like an odd requirement here.

Sure!

From the Qt's [source code](https://github.com/qt/qtbase/blob/92b685784960eea6eb353688cf0edeb94d69c6cd/cmake/QtBuildHelpers.cmake#L12-L16):
```cmake
message(WA
...

👍 dergoegge approved a pull request: "net: fuzz: bypass network magic and checksum validation"
(https://github.com/bitcoin/bitcoin/pull/31012#pullrequestreview-2354435611)
ACK d43966603bdefb0402a2dc93ff990cb254cdf2df

I think we could consider removing the honggfuzz netdriver instructions in a follow up but the changes look good to me anyway. I tested this by running honggfuzz in netdriver mode.

💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1791846656)
Looks like this was dropped entirely. Any particualr reason? It was previously was suppressing:
```bash
CMake Warning at qtbase/cmake/QtPublicAppleHelpers.cmake:920 (message):
Qt requires at least version 14 of Xcode, you're building against version .
Please upgrade. You can turn this warning into an error by configuring
with -DQT_FORCE_FATAL_APPLE_SDK_AND_XCODE_CHECK=ON.
Call Stack (most recent call first):
qtbase/cmake/QtBuildHelpers.cmake:443 (_qt_internal_check_apple_sdk_and_
...

💬 hebasto commented on pull request "build: Rename `PACKAGE_*` variables to `CLIENT_*`":
(https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2399840931)
Friendly ping @ryanofsky @theuni :)

💬 naumenkogs commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2399877567)
Would be great to address @vasild feedbacks :)

💬 marcofleon commented on issue "Disallow building fuzz binary without `-DBUILD_FOR_FUZZING`":
(https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2399886750)
This makes sense to me. Could be missing something, but I don't see a strong reason for building a fuzz binary without the specfic behaviors and optimizations we would like for fuzzing. If there's support for this, I could close https://github.com/bitcoin/bitcoin/pull/31028 and help out with this instead.

🤔 marcofleon reviewed a pull request: "net: fuzz: bypass network magic and checksum validation"
(https://github.com/bitcoin/bitcoin/pull/31012#pullrequestreview-2354553239)
Tested ACK d43966603bdefb0402a2dc93ff990cb254cdf2df

🚀 fanquake merged a pull request: "test: remove unused code from `script_tests`"
(https://github.com/bitcoin/bitcoin/pull/31051)

🚀 fanquake merged a pull request: "depends: For mingw cross compile use -gcc-posix to prevent library conflict"
(https://github.com/bitcoin/bitcoin/pull/31013)

👍 tdb3 approved a pull request: "test: Fix copy-paste in wallet/test/db_tests ostream operator"
(https://github.com/bitcoin/bitcoin/pull/31038#pullrequestreview-2354673777)
code review ACK f50557f5d36f568b7fe65ff5e242303b16b9b258

👍 fanquake approved a pull request: "ci: Double ctest timeout"
(https://github.com/bitcoin/bitcoin/pull/31056#pullrequestreview-2354673955)
ACK fa5ebc99207fb3dc9d052fbd489aa7abbaaf737f

🚀 fanquake merged a pull request: "ci: Double ctest timeout"
(https://github.com/bitcoin/bitcoin/pull/31056)

🚀 fanquake merged a pull request: "test: Fix copy-paste in wallet/test/db_tests ostream operator"
(https://github.com/bitcoin/bitcoin/pull/31038)

👍 instagibbs approved a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2354684021)
reACK 0b3ec8c59b2b6d598531cd4e688eb276e597c825

via `git range-diff master 2b21aebd9754c503bac12d1dbf566b3aa27451e8 0b3ec8c59b2b6d598531cd4e688eb276e597c825`

💬 fanquake commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2400014930)
MSAN failure was spurious, but not the tidy (https://cirrus-ci.com/task/4930868089716736):
```bash
[21:49:26.130] clang-tidy-18 -p=/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/src/validationinterface.cpp
[21:49:26.130] /ci_container_base/src/validationinterface.cpp:22:13: error: redundant 'RemovalReasonToString' declaration [readability-redundant-declaration,-warnings-as-errors]
[21:49:26.130] 22 | std::string Remov
...