fa780e1c25 build: Remove --enable-gprof (MarcoFalke)
Pull request description:
It is unclear what benefit this option has, given that:
* `gprof` requires re-compilation (`perf` and other tools can instead be used on existing executables)
* `gprof` requires hardening to be disabled
* `gprof` doesn't work with `clang`
* `perf` is documented in the dev-notes, and test notes, and embedded into the functional test framework; `gprof` isn't
* Anyone really wanting to use it could pass the required flags to `./configure`
* I couldn't find any mention of the use of `gprof` in the discussions in this repo, apart from the initial pull request adding it (cfaac2a60f)
* Keeping it means that it needs to be maintained and ported to CMake
Fix all issues by removing it.
ACKs for top commit:
TheCharlatan:
ACK fa780e1c25
hebasto:
ACK fa780e1c25, I have reviewed the code and it looks OK.
willcl-ark:
crACK fa780e1c25
Tree-SHA512: 0a9ff363ac2bec8b743878a4e3147f18bc16823d00c5007568432c36320bd0199b13b6d0ce828a9a83c2cc434c058afaa64eb2eccfbd93ed85b81ce10c41760c
Belt-and suspenders after #30234. Self-assignment should be safe _and_
discouraged.
We used to opt out of this warning because something deep in our
serialization/byteswapping code could self-assign, but that doesn't appear to
be the case anymore.
This supports lcov 2.x in the sense that we are no-longer hardcoding
version specific options, and instead will use the `LCOV_OPTS` variable
(which is the more correct/flexible thing to do in any case). It's also
quite likely that devs are already having to pass extra options to lcov
2.x, given it's more stringent in terms of coverage generation and error
checking. See this thread for an example:
https://github.com/linux-test-project/lcov/issues/238.
Added an example to the developer notes.
Tested on one machine (LCOV 2.0, gcc 13.2) with:
```bash
./autogen.sh
./configure --enable-lcov CXXFLAGS="-fprofile-update=prefer-atomic" LCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch"
make
make cov
<snip>
Processing file src/netaddress.cpp
lines=521 hit=480 functions=72 hit=72 branches=675 hit=499
Overall coverage rate:
lines......: 81.8% (79362 of 97002 lines)
functions......: 77.8% (10356 of 13310 functions)
branches......: 49.6% (130628 of 263196 branches)
```
and another machine (LCOV 2.1, Clang 18.1.3) with:
```bash
./autogen.sh
./configure --enable-lcov CC=clang CXX=clang++ LCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch,inconsistent"
make
make cov
<snip>
Processing file src/util/strencodings.cpp
lines=315 hit=311 functions=38 hit=38 branches=425 hit=357
Overall coverage rate:
source files: 622
lines.......: 79.8% (70311 of 88132 lines)
functions...: 78.1% (13968 of 17881 functions)
branches....: 44.5% (157551 of 354317 branches)
Message summary:
101 warning messages:
count: 1
inconsistent: 100
3528 ignore messages:
inconsistent: 3528
```
5deb0b024e build, test, doc: Temporarily remove Android-related stuff (Hennadii Stepanov)
Pull request description:
Previously, our Android builds were geared towards generating APKs, which relied on Qt. However, after migrating to C++20, compiling for Android became unfeasible due to Qt 5.15's compatibility limitations with NDK only up to r25, which includes an outdated embedded libc++ (see https://github.com/bitcoin/bitcoin/issues/29360).
All removed stuff will be reinstated after migrating the build system to CMake and upgrading Qt to version 6.x.
This PR makes possible a clean migration to the CMake-based build system as it removes code, which is not used at this moment.
ACKs for top commit:
vasild:
ACK 5deb0b024e
fanquake:
ACK 5deb0b024e - given none of this is currently tested/wont compile. Can be revisted in future.
Tree-SHA512: 3bc2ccfe881e11cc1d78c27acd6f1d86cfba86821ef3bb5eca2e80d978fdfa13659ec82284dcaadc507e2394524dea91d4b8f81d0030c1cef9708df8be76bf07
a057869aa3 build: pass --with-ecmult-gen-kb=86 to secp256k1 (fanquake)
ca3d945dc6 Squashed 'src/secp256k1/' changes from d8311688bd..06bff6dec8 (fanquake)
Pull request description:
This includes changes from the 0.5.0 release: https://github.com/bitcoin-core/secp256k1/releases/tag/v0.5.0
> New function secp256k1_ec_pubkey_sort that sorts public keys using lexicographic (of compressed serialization) order.
> The implementation of the point multiplication algorithm used for signing and public key generation was changed, resulting in improved performance for those operations.
> The related configure option --ecmult-gen-precision was replaced with --ecmult-gen-kb (ECMULT_GEN_KB for CMake).
> This changes the supported precomputed table sizes for these operations. The new supported sizes are 2 KiB, 22 KiB, or 86 KiB (while the old supported sizes were 32 KiB, 64 KiB, or 512 KiB).
ACKs for top commit:
hebasto:
ACK a057869aa3, I've got a zero diff with my local branch, which reproduces the subtree update, and `ecmult gen table size = 86 KiB` in the configure summary.
jonasnick:
utACK a057869aa3
Tree-SHA512: 907012b0d7e0a6bd68b245c238e968f2318d8ac5de5ec9070245de8391c996eb5ec6428184d028f6f0f54d3b2f5a8292ad7081177e1c331397879505436dc38e
Store the thread name in a `thread_local` variable of type `char[]`
instead of `std::string`. This avoids calling the destructor when
the thread exits. This is a workaround for
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=278701
For type-safety, return `std::string` from
`util::ThreadGetInternalName()` instead of `char[]`.
As a side effect of this change, we no longer store a reference
to a `thread_local` variable in `CLockLocation`. This was
dangerous because if the thread quits while the reference still
exists (in the global variable `lock_data`, see inside `GetLockData()`)
then the reference will become dangling.
Similar to libtool, (llvm-)otool only exists with a version suffix
on some systems (Ubuntu), which makes it annoying to use/find. Avoid
this, by switching to objdump. Which is a drop-in replacement.
This is related to #21778, and the switchover to using vanilla LLVM for
macOS.
Previously, our Android builds were geared towards generating APKs,
which relied on Qt. However, after migrating to C++20, compiling for
Android became unfeasible due to Qt 5.15's compatibility limitations
with NDK only up to r25, which includes an outdated embedded libc++.
All removed stuff will be reinstated after migrating the build system to
CMake and upgrading Qt to version 6.x."
Now that CXXFLAGS are back in user control, I don't think there's a
reason to no-longer use our warning flags when CXXFLAGS has been
overriden (this includes when building from depends).
Anyone can suppress warnings from third-party code by
passing the relevant `-Wno-` options in CXXFLAGS.
Fixes: #18092.
992c714451 common: Don't terminate on null character in UrlDecode (Fabian Jahr)
099fa57151 scripted-diff: Modernize name of urlDecode function and param (Fabian Jahr)
8f39aaae41 refactor: Remove hooking code for urlDecode (Fabian Jahr)
650d43ec15 refactor: Replace libevent use in urlDecode with our own code (Fabian Jahr)
46bc6c2aaa test: Add unit tests for urlDecode (Fabian Jahr)
Pull request description:
Fixes#29654 (as a side-effect)
Removing dependencies is a general goal of the project and the xz backdoor has been an additional wake up call recently. Libevent shows many of the same symptoms, few maintainers and slow releases. While libevent can not be removed completely over night we should start removing it’s usage where it's possible, ideally with the end goal to removing it completely.
This is a pretty easy win in that direction. The [`evhttp_uridecode` function from libevent](e0a4574ba2/http.c (L3542)) we were using in `urlDecode` could be easily emulated in fewer LOC. This also ports the [applicable test vectors over from libevent](https://github.com/libevent/libevent/blob/master/test/regress_http.c#L3430).
ACKs for top commit:
achow101:
ACK 992c714451
theStack:
Code-review ACK 992c714451
maflcko:
ACK 992c714451👈
stickies-v:
ACK 992c714451
Tree-SHA512: 78f76ae7ab3b6710eab2aaac20f55eb0da7803e057eaa6220e865f328666a5399ef1a479702aaf630b2f974ad3aa15e2b6adac9c11bc8c3d4be21e8af1667fea
d5a715536e build: remove boost::process dependency for building external signer support (Sebastian Falbesoner)
70434b1c44 external_signer: replace boost::process with cpp-subprocess (Sebastian Falbesoner)
cc8b9875b1 Add `cpp-subprocess` header-only library (Hennadii Stepanov)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/24907.
This PR is based on **theStack**'s [work](https://github.com/bitcoin/bitcoin/issues/24907#issuecomment-1466087049).
The `subprocess.hpp` header has been sourced from the [upstream repo](https://github.com/arun11299/cpp-subprocess) with the only modification being the removal of convenience functions, which are not utilized in our codebase.
Windows-related changes will be addressed in subsequent follow-ups.
ACKs for top commit:
achow101:
reACK d5a715536e
Sjors:
re-tACK d5a715536e
theStack:
Light re-ACK d5a715536e
fanquake:
ACK d5a715536e - with the expectation that this code is going to be maintained as our own. Next PRs should:
Tree-SHA512: d7fb6fecc3f5792496204190afb7d85b3e207b858fb1a75efe483c05260843b81b27d14b299323bb667c990e87a07197059afea3796cf218ed8b614086bd3611
2d1819455c crypto: chacha20: always use our fallback timingsafe_bcmp rather than libc's (Cory Fields)
Pull request description:
Looking at libc sources, apple and openbsd implementations match our naive fallback. Only FreeBSD (and only x86_64) seems to [implement an optimized version](https://github.com/freebsd/freebsd-src/blob/main/lib/libc/amd64/string/timingsafe_bcmp.S).
It's not worth the hassle of using a platform-specific function for such little gain.
Additionally, as mentioned below, this is the only case outside of sha2 that requires an autoconf check, and I have upcoming PRs to remove the sha2 ones.
Apple's [impl is unoptimized](https://opensource.apple.com/source/Libc/Libc-1244.1.7/string/FreeBSD/timingsafe_bcmp.c.auto.html).
As-is [OpenBSD's impl](https://github.com/openbsd/src/blob/master/lib/libc/string/timingsafe_bcmp.c).
Relevant IRC conversation with sipa:
> \<cfields\> sipa: chacha20poly1305.cpp uses libc's timingsafe_bcmp when possible. But looking around at apple/freebsd/openbsd, I don't see any impl that doesn't use the naive implementation that matches our fallback...
> \<cfields\> is there any reason to belive there's an optimized impl somewhere that we're actually hitting?
> \<cfields\> asking because after cleaning up sha2, timingsafe_bcmp is the last autoconf check that remains in all of crypto. It'd make life easy if we could just always use our internal one.
> \<cfields\> *all of crypto/
> \<sipa\> cfields: let's get rid of the dependency then
> \<sipa\> it's a trivial function
> \<sipa\> and if we need it for some platforms, no real reason not to use it on all
After the above discusstion, I did end up finding the x86_64-optimized FreeBSD impl, but I don't think that's all that significant.
ACKs for top commit:
sipa:
utACK 2d1819455c
fanquake:
ACK 2d1819455c
TheCharlatan:
ACK 2d1819455c
theStack:
ACK 2d1819455c
Tree-SHA512: b9583e19ac2f77c5d572aa5b95bc4b53669d5717e5708babef930644980de7c5d06a9c7decd5c2b559d70b8597328ecfe513375e3d8c3ef523db80012dfe9266
fa9f36baba build: Remove HAVE_GMTIME_R (MarcoFalke)
fa72dcbfa5 refactor: FormatISO8601* without gmtime* (MarcoFalke)
fa2c486afc Revert "time: add runtime sanity check" (MarcoFalke)
Pull request description:
Now that the `ChronoSanityCheck` has passed for everyone with C++17 and is guaranteed by C++20 to always pass, remove it.
Also, remove `gmtime_r` and `gmtime_s` and replace them with `year_month_day`+`hh_mm_ss` from C++20.
ACKs for top commit:
sipa:
utACK fa9f36baba
fanquake:
ACK fa9f36baba - more std lib & even less stuff to port.
Tree-SHA512: a9e7e805b757b7dade0bcc3f95273a7dc4f68622630d74838339789dd203ad7542d36b2e090a93b2bc5a7ecc383207dd7ec82c68147108bdac7ce44f088c8c9a
Looking at apple/freebsd/openbsd sources, their implementations match our naive
fallback. It's not worth the hassle of using a platform-specific function for
no gain.
567cec9a05 doc: add release notes and help text for unix sockets (Matthew Zipkin)
bfe5192891 test: cover UNIX sockets in feature_proxy.py (Matthew Zipkin)
c65c0d0163 init: allow UNIX socket path for -proxy and -onion (Matthew Zipkin)
c3bd43142e gui: accomodate unix socket Proxy in updateDefaultProxyNets() (Matthew Zipkin)
a88bf9dedd i2p: construct Session with Proxy instead of CService (Matthew Zipkin)
d9318a37ec net: split ConnectToSocket() from ConnectDirectly() for unix sockets (Matthew Zipkin)
ac2ecf3182 proxy: rename randomize_credentials to m_randomize_credentials (Matthew Zipkin)
a89c3f59dc netbase: extend Proxy class to wrap UNIX socket as well as TCP (Matthew Zipkin)
3a7d6548ef net: move CreateSock() calls from ConnectNode() to netbase methods (Matthew Zipkin)
74f568cb6f netbase: allow CreateSock() to create UNIX sockets if supported (Matthew Zipkin)
bae86c8d31 netbase: refactor CreateSock() to accept sa_family_t (Matthew Zipkin)
adb3a3e51d configure: test for unix domain sockets (Matthew Zipkin)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/27252
UNIX domain sockets are a mechanism for inter-process communication that are faster than local TCP ports (because there is no need for TCP overhead) and potentially more secure because access is managed by the filesystem instead of serving an open port on the system.
There has been work on [unix domain sockets before](https://github.com/bitcoin/bitcoin/pull/9979) but for now I just wanted to start on this single use-case which is enabling unix sockets from the client side, specifically connecting to a local Tor proxy (Tor can listen on unix sockets and even enforces strict curent-user-only access permission before binding) configured by `-onion=` or `-proxy=`
I copied the prefix `unix:` usage from Tor. With this patch built locally you can test with your own filesystem path (example):
`tor --SocksPort unix:/Users/matthewzipkin/torsocket/x`
`bitcoind -proxy=unix:/Users/matthewzipkin/torsocket/x`
Prep work for this feature includes:
- Moving where and how we create `sockaddr` and `Sock` to accommodate `AF_UNIX` without disturbing `CService`
- Expanding `Proxy` class to represent either a `CService` or a UNIX socket (by its file path)
Future work:
- Enable UNIX sockets for ZMQ (https://github.com/bitcoin/bitcoin/pull/27679)
- Enable UNIX sockets for I2P SAM proxy (some code is included in this PR but not tested or exposed to user options yet)
- Enable UNIX sockets on windows where supported
- Update Network Proxies dialog in GUI to support UNIX sockets
ACKs for top commit:
Sjors:
re-ACK 567cec9a05
tdb3:
re ACK for 567cec9a05.
achow101:
ACK 567cec9a05
vasild:
ACK 567cec9a05
Tree-SHA512: de81860e56d5de83217a18df4c35297732b4ad491e293a0153d2d02a0bde1d022700a1131279b187ef219651487537354b9d06d10fde56225500c7e257df92c1
These come from GUI code, and haven't/aren't being fixed, see discussion
in https://github.com/bitcoin-core/gui/issues/112. For now, just ignore
them entirely. Note that this only applies to ObjCXX code, so will not
hide any relevant warnings coming from C or CXX code (and they would be
unlikely in any case).
Alternative to #29362, which disables all compiler warnings, for macOS
builds in the CI.
Relevant output:
```bash
qt/macnotificationhandler.mm:27:9: warning: 'NSUserNotification' is deprecated: first deprecated in macOS 11.0 - All NSUserNotifications API should be replaced with UserNotifications.frameworks API [-Wdeprecated-declarations]
NSUserNotification* userNotification = [[NSUserNotification alloc] init];
^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSUserNotification.h:24:12: note: 'NSUserNotification' has been explicitly marked deprecated here
@interface NSUserNotification : NSObject <NSCopying> {
^
qt/macnotificationhandler.mm:27:50: warning: 'NSUserNotification' is deprecated: first deprecated in macOS 11.0 - All NSUserNotifications API should be replaced with UserNotifications.frameworks API [-Wdeprecated-declarations]
NSUserNotification* userNotification = [[NSUserNotification alloc] init];
^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSUserNotification.h:24:12: note: 'NSUserNotification' has been explicitly marked deprecated here
@interface NSUserNotification : NSObject <NSCopying> {
^
qt/macnotificationhandler.mm:30:11: warning: 'NSUserNotificationCenter' is deprecated: first deprecated in macOS 11.0 - All NSUserNotifications API should be replaced with UserNotifications.frameworks API [-Wdeprecated-declarations]
[[NSUserNotificationCenter defaultUserNotificationCenter] deliverNotification: userNotification];
^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSUserNotification.h:118:12: note: 'NSUserNotificationCenter' has been explicitly marked deprecated here
@interface NSUserNotificationCenter : NSObject {
^
3 warnings generated.
```
86b7f28d6c serialization: use internal endian conversion functions (Cory Fields)
432b18ca8d serialization: detect byteswap builtins without autoconf tests (Cory Fields)
297367b3bb crypto: replace CountBits with std::bit_width (Cory Fields)
52f9bba889 crypto: replace non-standard CLZ builtins with c++20's bit_width (Cory Fields)
Pull request description:
This replaces #28674, #29036, and #29057. Now ready for testing and review.
Replaces platform-specific endian and byteswap functions. This is especially useful for kernel, as it means that our deep serialization code no longer requires bitcoin-config.h.
I apologize for the size of the last commit, but it's hard to avoid making those changes at once.
All platforms now use our internal functions rather than libc or platform-specific ones, with the exception of MSVC.
Sadly, benchmarking showed that not all compilers are capable of detecting and optimizing byteswap functions, so compiler builtins are instead used where possible. However, they're now detected via macros rather than autoconf checks.
This[ matches how libc++ implements std::byteswap for c++23](https://github.com/llvm/llvm-project/blob/main/libcxx/include/__bit/byteswap.h#L26).
I suggest we move/rename `compat/endian.h`, but I left that out of this PR to avoid bikeshedding.
#29057 pointed out some irregularities in benchmarks. After messing with various compilers and configs for a few weeks with these changes, I'm of the opinion that we can't win on every platform every time, so we should take the code that makes sense going forward. That said, if any real-world slowdowns are caused here, we should obviously investigate.
ACKs for top commit:
maflcko:
ACK 86b7f28d6c📘
fanquake:
ACK 86b7f28d6c - we can finish pruning out the __builtin_clz* checks/usage once the minisketch code has been updated. This is more good cleanup pre-CMake & for the kernal.
Tree-SHA512: 715a32ec190c70505ffbce70bfe81fc7b6aa33e376b60292e801f60cf17025aabfcab4e8c53ebb2e28ffc5cf4c20b74fe3dd8548371ad772085c13aec8b7970e
1. It didn't actually disable asm usage in our code. Regardless of the setting,
asm is used in random.cpp and support/cleanse.cpp.
2. The value wasn't forwarded to libsecp as a user might have reasonably
expected.
3. We now have the DISABLE_OPTIMIZED_SHA256 define which is what disable-asm
actually did in practice.
If there is any desire, we can hook DISABLE_OPTIMIZED_SHA256 up to a new
configure option that actually does what it says.
ad7584d8b6 serialization: replace char-is-int8_t autoconf detection with c++20 concept (Cory Fields)
Pull request description:
Doesn't depend on #29263, but it's really only relevant after that one's merged.
This removes the only remaining autoconf macro in our serialization code (after #29263), so it can now be used trivially and safely out-of-tree.
~Our code does not currently contain any concepts, but couldn't find any discussion or docs about avoiding them. I guess we'll see if this blows up our c-i.~
Edit: Ignore this. ajtowns pointed out that we're already using a few concepts.
This was introduced in #13580. Please check my logic on this as I'm unable to test on a SmartOS system. Even better would be a confirmation from someone who can build there.
ACKs for top commit:
Empact:
Code review ACK ad7584d8b6
Tree-SHA512: 1faf65c900700efb1cf3092c607a2230321b393cb2f029fbfb94bc8e50df1dabd7a9e4b91e3b34f0d2f3471aaf18ee7e56d91869db5c5f4bae84da95443e1120
These replace our platform-specific mess in favor of c++20 endian detection
via std::endian and internal byteswap functions when necessary.
They no longer rely on autoconf detection.
Rather than a complicated set of tests to decide which bswap functions to
use, always prefer the compiler built-ins when available.
These builtins and fallbacks can all be removed once we're using c++23, which
adds std::byteswap.
This avoids cases of missing -O2, when *FLAGS has been overriden.
Removes the need for duplicate code to clear autoconf defaults.
Also, move CORE_CXXFLAGS before DEBUG_CXXFLAGS, so that -O2 is always
overriden if debugging etc.
2d1b1c7dae build: remove --enable-lto (fanquake)
Pull request description:
This has outlived its usefulness, doesn't gel well with newer compilers & `-flto` related options, i.e thin vs full, or `=auto`, and having `-flto` as the only option means that sometimes this just needs to be worked around, i.e in oss-fuzz:
https://github.com/google/oss-fuzz/blob/master/projects/bitcoin-core/build.sh.
While it was convenient when `-flto` was newer, support for `-flto` is now in all compilers we use, and there's also no-longer any real need for us to treat `-flto` different to any other optimization option.
Remove it, to remove build complexity, and so there's no need to port a similar option to CMake.
Note that the LTO option remains in depends, because we still a way to build packages that have LTO specific patches/options.
ACKs for top commit:
TheCharlatan:
ACK 2d1b1c7dae
hebasto:
ACK 2d1b1c7dae.
Tree-SHA512: 91812de7da35346f51850714a188fcffbac478bc8b348bf756c2555fcbde86ba622ac2fb77d294dea0378c741d3656f06121ef3a795aeed63fd170fc31bfa5af
aaaace2fd1 fuzz: Assume presence of __builtin_*_overflow, without checks (MarcoFalke)
fa223ba5eb Revert "build: Fix undefined reference to __mulodi4" (MarcoFalke)
fa7c751bd9 build: Bump clang minimum supported version to 14 (MarcoFalke)
Pull request description:
Most supported operating systems ship with clang-14 (or later), so bump the minimum to that and allow new code to drop workarounds for previous clang bugs.
For reference:
* https://packages.debian.org/bookworm/clang (`clang-14`)
* https://packages.ubuntu.com/jammy/clang (`clang-14`)
* CentOS-like 8/9 Stream: All Clang versions from 15 to 17
* FreeBSD 12/13: All Clang versions from 15 to 16
* OpenSuse Tumbleweed ships with https://software.opensuse.org/package/clang (`clang17`); No idea about OpenSuse Leap
On operating systems where the clang version is not shipped by default, the user would have to use GCC, or install clang in a different way. For example:
* https://packages.debian.org/bullseye/g++ (g++-10)
* https://packages.ubuntu.com/focal/g++-10
* https://apt.llvm.org/, or nix, or guix, or compile clang from source, ...
ACKs for top commit:
fanquake:
ACK aaaace2fd1
Tree-SHA512: 81d066b14cc568d27312f1cc814b09540b038a10a0a8e9d71fc9745b024fb6c32a959af673e6819b817ea7cef98da4abfa63dff16cffb7821b40083016b0291f
This has outlived its usefulness, doesn't gel well with
newer compilers & `-flto` related options, i.e thin vs full, or `=auto`,
and having `-flto` as the only option means that sometimes this just
needs to be worked around, i.e in oss-fuzz:
https://github.com/google/oss-fuzz/blob/master/projects/bitcoin-core/build.sh.
While it was convenient when `-flto` was newer, support for `-flto` is now
in all compilers we use, and there's also no-longer any real need
for us to treat `-flto` different to any other optimization option.
Remove it, to remove build complexity, and so there's no need
to port a similar option to CMake.
Note that the LTO option remains in depends, because we still a way to
build packages that have LTO specific patches/options.
If we decide to merge this, I'll follow up downstream in oss-fuzz first,
to make sure we don't break the build.
49a90915aa build: Bump minimum required Boost to 1.73.0 to support C++20 (Hennadii Stepanov)
Pull request description:
Boost versions <1.73 have C++20-specific bugs that were fixed in the following commits:
- 15fcf21356
- 495c095dc0
I tested [`libboost1.71-dev`](https://packages.ubuntu.com/focal/libboost1.71-dev) in Ubuntu 20.04 and Boost 1.71, 1.72, 1.73 in our depends build system.
Closes https://github.com/bitcoin/bitcoin/issues/29063.
ACKs for top commit:
fanquake:
ACK 49a90915aa
Tree-SHA512: b8ebc08af85abfa3fda70961bd1136ee9e5149dd76a3f901e43acba624d231971873cba5cbf30837f9e5ab58790b8330f241a76cb76d8cf5dce5ad0cca33fba8
308aec3e56 build: disable external-signer for Windows (fanquake)
35537318a1 ci: remove --enable-external-signer from win64 job (fanquake)
Pull request description:
It's come to light that Boost ASIO (a Boost Process sub dep) has in some
instances, been quietly initialising our network stack on Windows (see
PR https://github.com/bitcoin/bitcoin/pull/28486 and discussion in https://github.com/bitcoin/bitcoin/issues/28940).
This has been shielding a bug in our own code, but the larger issue
is that Boost Process/ASIO is running code before main, and doing things
like setting up networking. This undermines our own assumptions about
how our binary works, happens before we run any sanity checks,
and before we call our own code to setup networking. Note that ASIO also
calls WSAStartup with version `2.0`, whereas we call with `2.2`.
It's also not clear why a feature like external signer would have a
dependency that would be doing anything network/socket related,
given it only exists to spawn a local process.
See also the discussion in https://github.com/bitcoin/bitcoin/issues/24907. Note that the maintaince of Boost Process in general,
has not really improved. For example, rather than fixing bugs like https://github.com/boostorg/process/issues/111,
i.e, https://github.com/boostorg/process/pull/317, the maintainer chooses to just wrap exception causing overflows
in try-catch blocks: 0c42a58eac. These changes get merged in large,
unreviewed PRs, i.e https://github.com/boostorg/process/pull/319.
This PR disables external-signer on Windows for now. If, in future, someone
changes how Boost Process works, or replaces it entirely with some
properly reviewed and maintained code, we could reenable this feature on
Windows.
ACKs for top commit:
hebasto:
re-ACK 308aec3e56.
TheCharlatan:
ACK 308aec3e56
Tree-SHA512: 7405f7fc9833eeaacd6836c4e5b1c1a7845a40c1fdd55c1060152f8d8189e4777464fde650e11eb1539556a75dddf49667105987078b1457493ee772945da66e
