👍 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.
(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_
...
(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 :)
(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 :)
(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.
(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
(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)
(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)
(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
(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
(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)
(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)
(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`
(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
...
(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
...
🚀 fanquake merged a pull request: "test: Treat exclude list warning as failure in CI"
(https://github.com/bitcoin/bitcoin/pull/31018)
(https://github.com/bitcoin/bitcoin/pull/31018)
💬 fanquake commented on pull request "Add -pausebackgroundsync startup option":
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2400031610)
I agree with the other commentors here. I don't think we should be trying to extend/re-use a user-facing feature for development purposes, especially given the existence of more accurate/repeatable ways of achieving the same thing. Adding this option for the other uses-cases described just further murkies the function of/complicates this feature.
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2400031610)
I agree with the other commentors here. I don't think we should be trying to extend/re-use a user-facing feature for development purposes, especially given the existence of more accurate/repeatable ways of achieving the same thing. Adding this option for the other uses-cases described just further murkies the function of/complicates this feature.
🚀 fanquake merged a pull request: "ci: Add missing -DWERROR=ON to test-each-commit"
(https://github.com/bitcoin/bitcoin/pull/31045)
(https://github.com/bitcoin/bitcoin/pull/31045)
💬 joe-bp commented on issue "Embedded ASMap data Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2400037617)
> > Is there no way to query AS Maps directly from the BGP tables?
>
> Who's BGP table? Can you be a bit more specific what you are thinking of in terms of architecture and trust assumptions?
Hi Fabian, I've been talking to Gary about this a bit. I submitted an issue here: https://github.com/asmap/asmap-data/issues/17
I'd like to propose a way to pull in the actual real-time BGP routing table and use that data for asmap. Multiple people/orgs can do this independently and get the data
...
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2400037617)
> > Is there no way to query AS Maps directly from the BGP tables?
>
> Who's BGP table? Can you be a bit more specific what you are thinking of in terms of architecture and trust assumptions?
Hi Fabian, I've been talking to Gary about this a bit. I submitted an issue here: https://github.com/asmap/asmap-data/issues/17
I'd like to propose a way to pull in the actual real-time BGP routing table and use that data for asmap. Multiple people/orgs can do this independently and get the data
...
✅ fanquake closed a pull request: "doc: clarify 'filename' argument in 'loadwallet' RPC"
(https://github.com/bitcoin/bitcoin/pull/31031)
(https://github.com/bitcoin/bitcoin/pull/31031)
💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2400044072)
Force-pushed to rebase addressing a merge conflict, preserve `tfm::format_error` throwing behaviour for `DEBUG` builds to improve visibility into programming errors before they manifest, and [improve an error string](https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2389355374).
---
Thank you for your thoughtful review @ryanofsky. tl;dr I share your concerns, but I think I have a different opinion on the trade-offs made.
I think everyone here is in agreement that compile-time c
...
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2400044072)
Force-pushed to rebase addressing a merge conflict, preserve `tfm::format_error` throwing behaviour for `DEBUG` builds to improve visibility into programming errors before they manifest, and [improve an error string](https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2389355374).
---
Thank you for your thoughtful review @ryanofsky. tl;dr I share your concerns, but I think I have a different opinion on the trade-offs made.
I think everyone here is in agreement that compile-time c
...