9e5089dbb0 build, msvc: Enable `libqrencode` vcpkg package (Hennadii Stepanov)
30089b0cb6 cmake: Add `FindQRencode` module (Hennadii Stepanov)
Pull request description:
This PR introduces the `FindQRencode` CMake module, following the official CMake [guidelines](https://cmake.org/cmake/help/latest/manual/cmake-developer.7.html#find-modules) for managing [upstream libraries](https://github.com/fukuchi/libqrencode) that lack a config file package. This module enhances flexibility in locating the `libqrencode` library by making the use of `pkg-config` optional.
With this update, `libqrencode` can be detected on systems where either `pkg-config` or the `libqrencode.pc` file is unavailable, such as Windows environments using the vcpkg package manager. However, if `libqrencode.pc` is available, it remains beneficial as the only direct source of the library's version information.
Additionally, the `libqrencode` vcpkg package is enabled for MSVC builds.
Here is a diff for configuration output on Ubuntu 24.10:
```diff
-- Detecting CXX compile features - done
-- Found SQLite3: /usr/include (found suitable version "3.46.1", minimum required is "3.7.17")
-- Found PkgConfig: /usr/bin/pkg-config (found version "1.8.1")
--- Checking for module 'libqrencode'
--- Found libqrencode, version 4.1.1
+-- Found QRencode: /usr/lib/x86_64-linux-gnu/libqrencode.so (found version "4.1.1")
-- Found Qt: /usr/lib/x86_64-linux-gnu/cmake/Qt5 (found suitable version "5.15.15", minimum required is "5.11.3")
-- Performing Test CXX_SUPPORTS__WERROR
-- Performing Test CXX_SUPPORTS__WERROR - Success
```
ACKs for top commit:
fanquake:
ACK 9e5089dbb0
Tree-SHA512: bb9baca64386772f2f4752b1cbff1230792562ca6b2e37c56ad28580b55b1ae6ff65c2cf0d8ab026111d7b5a056d7ac672496a3cfd1a81e4fdd2b84c8cf75fff
fafbf8acf4 Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to execute a fuzz target (MarcoFalke)
fae3cf0ffa ci: Temporarily disable macOS/Windows fuzz step (MarcoFalke)
Pull request description:
`g_fuzzing` is used inside `Assume` at runtime, causing significant overhead in hot paths. See https://github.com/bitcoin/bitcoin/issues/31178
One could simply remove the `g_fuzzing` check from the `Assume`, but this would make fuzzing a bit less useful. Also, it would be unclear if `g_fuzzing` adds a runtime overhead in other code paths today or in the future.
Fix all issues by making `G_FUZZING` equal to the build option `BUILD_FOR_FUZZING`, and for consistency in fuzzing, require it to be set when executing any fuzz target.
Fixes https://github.com/bitcoin/bitcoin/issues/31178
Temporarily this drops fuzzing from two CI tasks, but they can be re-added in a follow-up with something like https://github.com/bitcoin/bitcoin/pull/31073
ACKs for top commit:
marcofleon:
Tested ACK fafbf8acf4
davidgumberg:
I still ACK fafbf8acf4 for fixing the regression measured in #31178.
ryanofsky:
Code review ACK fafbf8acf4 but approach -0, because this approach means libraries built for fuzz testing do not function correctly if used in a release, and libraries built for releases are mostly useless for fuzz testing. So I would like to at least consider other solutions to this problem even if we go with this one.
dergoegge:
utACK fafbf8acf4
Tree-SHA512: 124fc2e8b35e0c4df414436556a7a0a36cd1bec4b3000b40dcf2ab8c85f32e0610bf7f70d2fd79223d62f3c3665b6c09da21241654c7b9859461b8ca340d5421
2957ca9611 build: have "make test" depend on "make all" (Cory Fields)
Pull request description:
See [Upstream docs](https://cmake.org/cmake/help/latest/variable/CMAKE_SKIP_TEST_ALL_DEPENDENCY.html) for specifics.
Unfortunately, this **seems to have no effect when directly executing `ctest`** :(
This brings the test -> hack -> test cycle more inline with how it worked with autotools.
With `CMAKE_SKIP_TEST_ALL_DEPENDENCY` set to FALSE, `make test` will trigger a rebuild, ensuring that test binaries are current before running them.
To test:
```
cmake -S . -B build
make -C build -j24
touch src/primitives/transaction.cpp
make -C build test ARGS=-j24
```
Without this commit, the above will not rebuild before running tests.
ACKs for top commit:
tdb3:
ACK 2957ca9611
itornaza:
ACK 2957ca9611
laanwj:
ACK 2957ca9611
Tree-SHA512: 46e6d53fd9c28b2f47b34c42ae0b061b01ef7614cdf5ee7d49fb9d3413ca92983d8a996379a9067ac8a8cfc16b230946401e68cf114bd4e3c5c02db2c55ec6bb
915640e191 depends: zeromq: don't install .pc files and remove patches for them (Cory Fields)
6b8a74463b cmake: Add `FindZeroMQ` module (Hennadii Stepanov)
Pull request description:
This PR introduces the `FindZeroMQ` module, which first attempts to find the `libzmq` library using CMake's `find_package()` and falls back to `pkg_check_modules()` if unsuccessful.
Addresses https://github.com/bitcoin/bitcoin/issues/30876 for the ZeroMQ package.
ACKs for top commit:
fanquake:
ACK 915640e191
Tree-SHA512: 2f17bae21be5d3f280a13425d22f5d1b2e23837a8aaf5ec89c433767509de030a42d598b261e102bdb5b860d8ede98013c124c3d25e081e956d4ee3a81b2584f
The use of `PACKAGE_NAME` for the project's variable name is
problematic, as this name is commonly used in CMake's interface
variables. If third-party CMake code handles with scopes improperly,
our `PACKAGE_NAME` variable could end up with an unexpected value.
This change avoids such conflicts by renaming all `PACKAGE_*` variables
to `CLIENT_*`.
9f243cd7fa Introduce `g_fuzzing` global for fuzzing checks (dergoegge)
Pull request description:
This PR introduces a global `g_fuzzing` that indicates if we are fuzzing.
If `g_fuzzing` is `true` then:
* Assume checks are enabled
* Special fuzzing paths are taken (e.g. pow check is reduced to one bit)
Closes#30950#31057
ACKs for top commit:
maflcko:
review ACK 9f243cd7fa 🗜
brunoerg:
crACK 9f243cd7fa
marcofleon:
Tested ACK 9f243cd7fa
Tree-SHA512: 56e4cad0555dec0c565ea5ecc529628ee4f37d20dc660c647fdc6948fbeed8291e6fe290de514bd4c2c7089654d9ce1add607dc9855462828b62be9ee45e4999
fa1b139d17 Bump python minimum supported version to 3.10 (MarcoFalke)
Pull request description:
All supported operating systems ship with python 3.10 (or later), so bumping the minimum should not cause any issues. A bump will allow new code to use new python features.
For reference:
* https://packages.debian.org/bookworm/python3
* https://packages.ubuntu.com/jammy/python3
* FreeBSD 13/14 ships with 3.11
* CentOS-like 8/9 ships with 3.11/3.12 (via `appstream`)
* OpenSuse Tumbleweed ships with all python versions, e.g. https://software.opensuse.org/package/python312-base
This is for Bitcoin Core 29.0 in 2025 (next year), not the soon upcoming 28.0 this fall.
ACKs for top commit:
achow101:
ACK fa1b139d17
AngusP:
ACK fa1b139d17
l0rinc:
ACK fa1b139d17
stickies-v:
ACK fa1b139d17
Tree-SHA512: 910b202ff2374bb21c93e5249a151fd2c8f63759bed5659676b0e467afa6e8e977be797c3fccceca303c82575e11ec236a8d7c5880910e4314b3875b820e7e8a
a97f43d63a fuzz: Add harness for p2p headers sync (marcofleon)
a0eaa4749f Add FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION in PoW check (marcofleon)
a3f6f5acd8 build: Automatically define FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION for fuzz builds (marcofleon)
0c02d4b2bd net_processing: Make MAX_HEADERS_RESULTS a PeerManager option (marcofleon)
Pull request description:
This PR reopens https://github.com/bitcoin/bitcoin/pull/28043. It's a regression fuzz test for https://github.com/bitcoin/bitcoin/pull/26355 and [a couple bugs](ed6cddd98e) that were addressed in https://github.com/bitcoin/bitcoin/pull/25717. This should help us move forward with the [removal of mainnet checkpoints](https://github.com/bitcoin/bitcoin/pull/25725).
It seems like the main concern in https://github.com/bitcoin/bitcoin/pull/28043 was the global mock function for proof of work. This PR aims to be an improvement by replacing the previous approach with a fuzz build configured using `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION`. This ensures that the simplified test code will never be in a release binary. If we agree this is the way to go, there are some other places (for future targets) where this method could be used.
In this target, PoW isn't being tested, so the goal is to bypass the check and let the fuzzer do its thing. In the other harnesses where PoW is actually being fuzzed, `CheckProofOfWork` is now `CheckProofOfWorkImpl`. So, the only change to that function is in the name.
More about `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` can be found at https://llvm.org/docs/LibFuzzer.html#fuzzer-friendly-build-mode and https://github.com/AFLplusplus/AFLplusplus/blob/stable/docs/fuzzing_in_depth.md#d-modifying-the-target.
ACKs for top commit:
naumenkogs:
ACK a97f43d63a
dergoegge:
reACK a97f43d63a
instagibbs:
tested ACK a97f43d63a
brunoerg:
ACK a97f43d63a
Tree-SHA512: 60b0bc6aadd8ca4c39db9cbba2da2debaaf68afcb6a8dd75c1ce48ca9e3996948fda8020930b6771a424e0f7c41b0b1068db4aa7dbe517f8fc152f1f712058ad
The removed commands were left over from the transition from
autodetection to explicit options. These commands prevented the
`-DENABLE_WALLET=OFF` option from being work properly when building with
depends.
30803a35d5 cmake: decouple FORTIFY_SOURCE check from Debug build type (fanquake)
Pull request description:
`FORTIFY_SOURCE` should be used if `ENABLE_HARDENING=ON` and optimisations are being used. This should not be coupled to any particular build type, because even if the build type is `Debug`, optimisations might still be in use.
Fixes: #30800.
Also somewhat of a followup to https://github.com/bitcoin/bitcoin/pull/30778#discussion_r1742257436.
ACKs for top commit:
ryanofsky:
Code review ACK 30803a35d5
TheCharlatan:
ACK 30803a35d5
Tree-SHA512: 298f8805a5bb2f1ff54e51ea31324d712c2070cc3eba26561c31001ace4bfa37ae6d18531cbd45e2faf610a0a1b83b420fcde6e329e17f02b021d26563583913
`FORTIFY_SOURCE` should be used if `ENABLE_HARDENING=ON` and optimisations
are being used. This should not be coupled to any particular build type,
because even if the build type is `Debug`, optimisations might still
be in use.
Fixes: #30800.