7b00595d33 build: stop overriding user CXXFLAGS (fanquake)
3e2ef23c3e build: stop overriding user LDFLAGS (fanquake)
35c3fd43c3 build: stop overriding user CPPFLAGS (fanquake)
bc7cc57607 doc: explain why we clear CXXFLAGS with enable-debug (fanquake)
Pull request description:
Historically our build system has hijacked `CXXFLAGS` and friends, and this has always been a source of complaints from users and developers. With this PR, we move away from using `CXXFLAGS`, `CPPFLAGS` and `LDFLAGS`, and instead use `CORE_*FLAGS` variables for our flags / options, leaving autoconfs `FLAG` vars to the user.
Note that there are currently two cases where we will at least clear `CXXFLAGS` (if not alreaddy overridden by the user), when doing debugging or when coverage is enabled, to avoid Autoconfs `-g -O2` CXXFLAG default.
ACKs for top commit:
hebasto:
ACK 7b00595d33
Tree-SHA512: bda936a7aa8f98a1bf1552306845cb4bbab54e19a7a0b9ce3210e10fef70db146e9fe42a0cc8c50b2908506771b5b96f39c334e41323b70ec878e4010373096c
0d01272cd8 build: don't use Boost multi_index serialization (fanquake)
Pull request description:
We don't use the serialization or archiving facilities of multi_index.
So globally disable support, which gives a minor improvement in build
time, i.e less preprocessing work, given we don't link any Boost libs.
See: https://www.boost.org/doc/libs/1_78_0/libs/multi_index/doc/tutorial/creation.html
> Serialization capabilities are automatically provided by just linking with the appropriate Boost.Serialization library module: it is not necessary to explicitly include any header from Boost.Serialization, apart from those declaring the type of archive used in the process. If not used, however, serialization support can be disabled by globally defining the macro BOOST_MULTI_INDEX_DISABLE_SERIALIZATION. Disabling serialization for Boost.MultiIndex can yield a small improvement in build times, and may be necessary in those defective compilers that fail to correctly process Boost.Serialization headers.
ACKs for top commit:
MarcoFalke:
cr ACK 0d01272cd8
Tree-SHA512: 87c664a2f142dc6b8f8598341f9829be3fda8cf614d73cc9a894c8033ee40c6daa9b50f4049ecb1f1e3aaf342568d9a5f5c65af1e04c36ee3a9cb46eca95767b
d4ba2b2cbc compat: remove strnlen back-compat code (fanquake)
Pull request description:
This was needed for mingw (not mingw-w64), and some older versions of
macOS, which we no-longer support.
ACKs for top commit:
hebasto:
ACK d4ba2b2cbc
Tree-SHA512: d1beb9df58464feea3076091361d7d46e4a8901e347644a5fa6f24e052ca24ee0c7c0dd3f2a3d682b0204bf50430fa89eac62121691ea08af6dcf6b907bdec87
532c64a726 build: Fix Boost.Process test for Boost 1.78 (Hennadii Stepanov)
Pull request description:
Rebased #24415 with Luke's suggestion.
Fixes#24413.
ACKs for top commit:
hebasto:
ACK 532c64a726, tested on Mac mini (M1, 2020) + macOS Monterey 12.3 (21E230).
Tree-SHA512: 74f779695f6bbc45a2b7341a1402f747cc0d433d74825c7196cb9f156db0c0299895365f01665bd0bff12a8ebb5ea33a29b9a52f5eac0007ec35d1dca6544705
5a157eb370 Bugfix: configure: Only avoid -isystem for exact /usr/include path (Luke Dashjr)
556ee6f2fa Bugfix: configure: Quote SUPPRESS_WARNINGS sufficiently to preserve brackets (Luke Dashjr)
Pull request description:
The regex includes `[/ ]` which is supposed to match either a forward slash or a space, but m4 treats the brackets as special characters and effectively strips them out, leading to -isystem /usr/include paths except for in the typical scenario where it is the final parameter in the flag string.
ACKs for top commit:
hebasto:
ACK 5a157eb370, tested on Ubuntu 22.04 with clang 14.0.
vasild:
ACK 5a157eb370
Tree-SHA512: 5c8c282b647b7853b8fad1b5b473703c4a0635073d2685a8ac984151046e2c6a859e6972465419d27356dd29a47f21a2a3a6ad402ec434fe1f9882e5a35f0749
999982b06c build: Add --enable-c++20 option (MarcoFalke)
fae679065e Add CSerializedNetMsg::Copy() helper (MarcoFalke)
fabb7c4ba6 Make fs.h C++20 compliant (MarcoFalke)
fae2220f4e scheduler: Capture ‘this’ explicitly in lambda (MarcoFalke)
Pull request description:
This is for CI and devs only and doesn't change that C++17 is the standard we are currently using. The option `--enable-c++20` allows CI to check that the C++17 code in the repo is also valid C++20. (There are some cases where valid C++17 doesn't compile under C++20).
Also, it allows developers to easily play with C++20 in the codebase.
ACKs for top commit:
ryanofsky:
Code review ACK 999982b06c. Since last review was rebased, and enum-conversion change was dropped, and CSerializedNetMsg copy workaround was added
fanquake:
utACK 999982b06c
Tree-SHA512: afc95ba03ea2b937017fc8e2b1449379cd2b6f7093c430d2e344c665a00c51e402d6651cbcbd0be8118ea1e54c3a86e67d2021d19ba1d4da67168e9fcb6b6f83
The regex includes [/ ] which is supposed to match either a forward slash or a
space, but m4 treats the brackets as special characters and effectively strips
them out, leading to -isystem /usr/include paths except for in the typical
scenario where it is the final parameter in the flag string.
2c03cec2ff ci: Build bitcoin-chainstate (Carl Dong)
095aa6ca37 build: Add example bitcoin-chainstate executable (Carl Dong)
Pull request description:
Part of: #24303
This PR introduces an example/demo `bitcoin-chainstate` executable using said library which can print out information about a datadir and take in new blocks on stdin.
Please read the commit messages for more details.
-----
#### You may ask: WTF?! Why is `index/*.cpp`, etc. being linked in?
This PR is meant only to capture the state of dependencies in our consensus engine as of right now. There are many things to decouple from consensus, which will be done in subsequent PRs. Listing the files out right now in `bitcoin_chainstate_SOURCES` is purely to give us a clear picture of the task at hand, it is **not** to say that these dependencies _belongs_ there in any way.
### TODO
1. Clean up `bitcoin-chainstate.cpp`
It is quite ugly, with a lot of comments I've left for myself, I should clean it up to the best of my abilities (the ugliness of our init/shutdown might be the upper bound on cleanliness here...)
ACKs for top commit:
ajtowns:
ACK 2c03cec2ff
ryanofsky:
Code review ACK 2c03cec2ff. Just rebase, comments, formatting change since last review
MarcoFalke:
re-ACK 2c03cec2ff 🏔
Tree-SHA512: 86e7fb5718caa577df8abc8288c754f4a590650d974df9d2f6476c87ed25c70f923c4db651c6963f33498fc7a3a31f6692b9a75cbc996bf4888c5dac2f34a13b
On the master branch, bump to 23.99 (pre-24.0).
Tree-SHA512: 1e3b0cee8a2b5080170b59a4c445a3c1b69b99152e8eec7eba7080ab447cc6f9c6bd8f69df2b18ee9416de44a6ed88009a200ad26e89275f6230339330d12314
774323e378 ci: Force `--enable-external-signer` to prevent future regressions (Hennadii Stepanov)
69978858a4 build: Fix Boost.Process check for Boost 1.73 and older (Hennadii Stepanov)
2199ef79cb build: Fix a non-portable use of `test` (Hennadii Stepanov)
d436c488d4 build, refactor: Replace tabs with spaces (Hennadii Stepanov)
Pull request description:
On master (5f44c5c428) Boost.Process check false fails without the `-lpthread` flag.
```
$ grep -C 2 pthread_detach config.log
/usr/bin/ld: /tmp/cczCQfQv.o: in function `boost::asio::detail::posix_global_impl<boost::asio::system_context>::~posix_global_impl()':
conftest.cpp:(.text._ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED2Ev[_ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED5Ev]+0xa3): undefined reference to `pthread_join'
/usr/bin/ld: conftest.cpp:(.text._ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED2Ev[_ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED5Ev]+0xc4): undefined reference to `pthread_detach'
collect2: error: ld returned 1 exit status
configure:26674: $? = 1
```
Not required for Boost 1.74+.
ACKs for top commit:
laanwj:
Code review ACK 774323e378, is a bugfix/workaround, seems fine to merge last minute for 23.0.
Tree-SHA512: 2a9d4b67fd8910e107af972d8c223286b7c933bc310616f86c8b6d8c903438916980fc76bd7e37f2698f6ce5361dc706cbf2933d1ac2c081bcabe1b83ca7d6b6
faef344f84 Print enable_fuzz_binary in configure (MarcoFalke)
Pull request description:
A *disabled* `enable_fuzz` on current master does *not* mean the the fuzz binary is not compiled. This is confusion, so fix it.
* `enable_fuzz` toggles compilation flags for fuzzing and disables all other target. There is no need to print this in the configure result, because the compilation flags are already printed. Also, all other targets are already printed as `no`.
* `enable_fuzz_binary` does what it says it does and is currently not printed. So print it.
ACKs for top commit:
hebasto:
ACK faef344f84, tested on Linux Mint 20.2 (x86_64):
Tree-SHA512: 9b02b05c4b9c5fc92cf3487497392690303c36eace5e217f18b4349f059b5a23a7c0e0d030fb6fa7bbad83e927576a5e81c00099164f9ed8e185c0969dc17689
abc057c603 build: Add Boost.Process usage check (Hennadii Stepanov)
Pull request description:
This PR adds a check that Boost.Process can be used without linking any libraries (header-only).
Disable the functionality if that is not the case.
Fixesbitcoin/bitcoin#24314.
ACKs for top commit:
fanquake:
ACK abc057c603
Tree-SHA512: ed2a32b1f751ec6f88cc7220766edd4cdab93c1d0515c892aa3094ee8d5b13ef569830d6e7a7a00c0197b117585dc526d00d943cc99a1f8c8a66ac4e20fe2061
The bitcoin-chainstate executable serves to surface the dependencies
required by a program wishing to use Bitcoin Core's consensus engine as
it is right now.
More broadly, the _SOURCES list serves as a guiding "North Star" for the
libbitcoinkernel project: as we decouple more and more modules of the
codebase from our consensus engine, this _SOURCES list will grow shorter
and shorter. One day, only what is critical to our consensus engine will
remain. Right now, it's "the minimal list of files to link in to even
use our consensus engine".
[META] In a future commit the libbitcoinkernel library will be extracted
from bitcoin-chainstate, and the libbitcoinkernel library's
_SOURCES list will be the list that we aim to shrink.
0c49e52b22 build: remove unneeded getentropy detection (HAVE_GETENTROPY) (Sebastian Falbesoner)
5cd15ffdce random: use arc4random on OpenBSD (Sebastian Falbesoner)
Pull request description:
Inspired by a discussion on obtaining randomness on various OSes in a secp256k1 PR (https://github.com/bitcoin-core/secp256k1/pull/748#discussion_r524605472, see also https://bitcoincore.reviews/libsecp256k1-748), I think it makes sense to follow best practices and use `arc4random_buf` rather than `getentropy` on OpenBSD in our random module.
The [getentropy(2) man page](https://man.openbsd.org/getentropy.2) states:
```
getentropy() is not intended for regular code; please use the
arc4random(3) family of functions instead.
```
The [arc4random(3) man page](https://man.openbsd.org/arc4random.3) states:
```
Use of these functions is encouraged for almost all random number
consumption because the other interfaces are deficient in either quality,
portability, standardization, or availability.
```
On the linked PR discussion worries about using RC4 internally has been expressed (see https://security.stackexchange.com/questions/85601/is-arc4random-secure-enough/172905#172905), but this would only affect users of OpenBSD <5.5, using a version that was released more than 8 years ago.
ACKs for top commit:
laanwj:
Tested ACK 0c49e52b22
Tree-SHA512: b5ed3d0718962c5a3839db9a28f93d08a0ac93094cc664f83bc4cf1cfad25049e6240b7b81fe06b71e6a3a0ca24a2c337eab088abec5470ad014e10c04fdb216
c62d763fc3 Necessary improvements to make configure work without libevent installed (Perlover)
091ccc38c2 The evhttp_connection_get_peer function from libevent changes the type of the second parameter. Fixing the problem. (Perlover)
Pull request description:
The second parameter of evhttp_connection_get_peer in libevent already has type as `const char **`
The compilation of bitcoind with the fresh libevent occurs errors
Details: https://github.com/bitcoin/bitcoin/issues/23606
ACKs for top commit:
laanwj:
Code review ACK c62d763fc3
luke-jr:
tACK c62d763fc3
Tree-SHA512: d1c8062d90bd0d55c582dae2c3a7e5ee1b6c7ca872bf4aa7fe6f45a52ac4a8f59464215759d961f8efde0efbeeade31b08daf9387d7d50d7622baa1c06992d83
6200fbf54f build: rename --enable-ebpf to --enable-usdt (0xb10c)
e158a2a7aa build: add systemtap's sys/sdt.h as depends (0xb10c)
Pull request description:
There has been light conceptual agreement on including the Userspace, Statically Defined Tracing tracepoints in Bitcoin Core release builds. This, for example, enables user to hook into production deployments, if they need to. Binaries don't have to be switched out. This is possible because we don't do [expensive computations](https://github.com/bitcoin/bitcoin/blob/master/doc/tracing.md#no-expensive-computations-for-tracepoints) only needed for the tracepoints. The tracepoints are NOPs when not used.
Systemtap's `sys/sdt.h` header is required to build Bitcoin Core with USDT support. The header file defines the `DTRACE_PROBE` macros used in [`src/util/trace.h`](https://github.com/bitcoin/bitcoin/blob/master/src/util/trace.h). This PR adds Systemtap 4.5 (May 2021) as dependency. GUIX builds for Linux hosts now include the tracepoints.
Closes https://github.com/bitcoin/bitcoin/issues/23297.
ACKs for top commit:
fanquake:
ACK 6200fbf54f - tested enabling / disabling and with/without SDT from depends. We can follow up with #23819, #23907 and #23296, and if any serious issues arise before feature freeze, it is easy for us to flip depends such that USDT becomes opt-in, rather than opt-out, and thus, releases would be tracepoint free.
Tree-SHA512: 0263f44892bf8450e8a593e4de7a498243687f8d81269e1c3283fa8354922c7cf93fddef4b92cf5192d33798424aa5812e03e68ef8de31af078a32dd34021382
eBPF is a Linux kernel technology used to "extend the capabilities
of the kernel without requiring to change kernel source code or
load kernel modules". While Userspace, Statically Defined Tracing
(USDT) uses eBPF under the hood, --enable-usdt better resembles that
support for USDT is enabled, and tracepoints will be included in the
binary.
e09773d20a build: use a static .tiff for macOS .dmg over generating (fanquake)
Pull request description:
For demonstration, after [discussion in #23778](https://github.com/bitcoin/bitcoin/pull/23778#issuecomment-1003005503), and the question as to why we can't just have a `background.tiff` that we copy into the macOS DMG, and do away with the somewhat convoluted image generation steps.
From my understanding, the only reason we have this image generation as part of our build system is so that forks of Core can adapt the imagery for their own branding via `PACKAGE_NAME`. It don't think it provides much value to us, and could just have a static .tiff that we copy into the dmg (replacing the .svg that currently lives in macdeploy/).
Doing this would eliminate the following build dependencies:
For native macOS:
* `sed` (usage in Makefile.am)
* `librsvg` (rsvg-convert)
* `tiffutil`
Linux macOS cross-compile:
* `sed` (usage in Makefille.am)
* `librsvg`
* `tiffcp`
* `convert` (imagemagick)
* `font-tuffy`
Guix Build:
```bash
bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
c98d67796863f4b1bab0ad600d46bd74e744d94072cbd4bc856a6aeaba3bb329 guix-build-e09773d20a92/output/dist-archive/bitcoin-e09773d20a92.tar.gz
3336f90bab312798cb7665e2b4ae24d1a270fb240647d5fed8dbfcd83e3ed37e guix-build-e09773d20a92/output/x86_64-apple-darwin/SHA256SUMS.part
8fd680c7ee158c64bad212385df7b0b302c6c2143d4e672b4b0eb5da41f9256d guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx-unsigned.dmg
34f54177c2f0700e8cfaf5d85d91e404807cd9d411e22006cdff82653e5f4af2 guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx-unsigned.tar.gz
da6b8f54ef755d40330c8eac4f5bd0329637e827be9ee61318600d5d0bdcc3dc guix-build-e09773d20a92/output/x86_64-apple-darwin/bitcoin-e09773d20a92-osx64.tar.gz
```
ACKs for top commit:
hebasto:
ACK e09773d20a
jarolrod:
ACK e09773d20a
Zero-1729:
ACK e09773d20a
Tree-SHA512: 0ad06699a5451daa8cfaaa46759eb7bd85254a72e23f857f70d433a2ffb1a4bf6dd464d9c4ac9f8c20aab045f4e2b61c6dcdcbcceef96ce515b1a0c501665b1f
1bf3809dd1 build: use host_os instead of TARGET_OS in configure output (fanquake)
Pull request description:
`TARGET_OS` was convenient, as a readable host name for most of our
targeted platforms, however unless we add more code to configure to
detect more hosts, it's easier just use `host_os` (it's also more
informative).
i.e FreeBSD master
```bash
target os =
build os = freebsd13.0
```
this PR:
```bash
target os = freebsd13.0
build os = freebsd13.0
```
ACKs for top commit:
hebasto:
ACK 1bf3809dd1
Tree-SHA512: 606d92f60ce3f2f6ab1f54e29b5c179048c62ba51336b272c081b1e009128dd83705b181cfe30991c7a51d9c63e8ba2076bfed9e6112e7d1a74a7f947c5754f5
fanquake
