415650cea94f5050d7c368fdf9fd9878809957e1 guix: move *-check.py scripts under contrib/guix (fanquake)
Pull request description:
These scripts are not meant for general developer usage. They are for use on the release binaries, which have been compiled in an environment that makes various assumptions in regards to c library, compiler options, hardening options, dependency patching etc.
Anyone is free to run these scripts against self-compiled binaries, but this isn't something we want to modify/generalize the scripts to support.
ACKs for top commit:
laanwj:
Code review ACK 415650cea94f5050d7c368fdf9fd9878809957e1
TheCharlatan:
ACK 415650cea94f5050d7c368fdf9fd9878809957e1
Tree-SHA512: 469cc5da9df014b4a9dc39080f31c3283641691209497cdb7cc5dc4767681a7f4b67425768e81838153ce39a26547ed94cd3be4dc27352d33960765bfac9d0e5
a58cb3b1c12c8cb75a87375c50f94c4605bb805d qa: sanity check mined block have their coinbase timelocked to height (Antoine Poinsot)
8f2078af6a55448c003b3f7f3021955fbb351caa miner: timelock coinbase transactions (Antoine Poinsot)
788aeebf343526760fa8f3ed969ac3713212a5b6 qa: use prev height as nLockTime for coinbase txs created in unit tests (Antoine Poinsot)
c76dbe9b8b6f03b761a0ef97e1b8cd133b934714 qa: timelock coinbase transactions created in fuzz targets (Antoine Poinsot)
9c94069d8b6cf67a24eb03c51230a4f2b2bf2d64 contrib: timelock coinbase transactions in signet miner (Antoine Poinsot)
a5f52cfcc400ad0adb41a78c65b8abb971e0d622 qa: timelock coinbase transactions created in functional tests (Antoine Poinsot)
Pull request description:
The Consensus Cleanup soft fork proposal includes enforcing that coinbase transactions set their
nLockTime field to the block height minus 1, as well as their nSequence such as to not disable the
timelock. If such a fork were to be activated by Bitcoin users, miners need to be ready to produce
compliant blocks at the risk of losing substantial amounts mining would-be invalid blocks. As miners
are unfamously slow to upgrade, it's good to make this change as early as possible.
Although Bitcoin Core's GBT implementation does not provide the `coinbasetxn` field, and mining
pool software crafts the coinbase on its own, updating the Bitcoin Core mining code is a first step
toward convincing pools to update their (often closed source) code. A possible followup is also to
introduce new fields to GBT. In addition, this first step also makes it possible to test future
Consensus Cleanup changes.
The commit making the change also updates a bunch of seemingly-unrelated tests. This is because those tests were asserting error messages based on the txid of transactions involved, and changing the coinbase transaction structure necessarily changes the txid of all tests' transactions.
ACKs for top commit:
Sjors:
Code review ACK a58cb3b1c12c8cb75a87375c50f94c4605bb805d
achow101:
ACK a58cb3b1c12c8cb75a87375c50f94c4605bb805d
TheCharlatan:
Re-ACK a58cb3b1c12c8cb75a87375c50f94c4605bb805d
Tree-SHA512: a2aae009a187eb760d34435f518a895ee76c6b02a667eb030ddf6bd584da6e8eae2737d974dbf81a928d60c07bcb4820f055adc067e18d8819640db0240bb513
These scripts are not meant for general developer usage. They are for
use on the release binaries, which have been compiled in an environment
that makes various assumptions in regards to c library, compiler
options, hardening options, patching etc.
84de8c93e7d4979575161a2bb8f7eb64e1317b89 ci: Add `deploy` target for native macOS CI job (Hennadii Stepanov)
fad57e9e0fe2b57e9569aa26307c6344522c2a43 build: Fix `macdeployqtplus` after switching to Qt 6 (Hennadii Stepanov)
938208d91a27e3354f42eedbd6c5bf26117c4610 build: Resolve `@rpath` in `macdeployqtplus` (Hennadii Stepanov)
Pull request description:
Homebrew's Qt 6 package — namely `qt` or `qt@6` — introduces a few differences that must be properly handled by the `macdeployqtplus` script:
1. Use of `@rpath` references:
```
% objdump --macho --dylibs-used $(brew --prefix qt@5)/Frameworks/QtGui.framework/QtGui
/usr/local/opt/qt@5/Frameworks/QtGui.framework/QtGui:
/usr/local/opt/qt@5/lib/QtGui.framework/Versions/5/QtGui (compatibility version 5.15.0, current version 5.15.16)
/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit (compatibility version 45.0.0, current version 2575.30.19)
/System/Library/Frameworks/Metal.framework/Versions/A/Metal (compatibility version 1.0.0, current version 367.6.0)
/usr/local/Cellar/qt@5/5.15.16_1/lib/QtCore.framework/Versions/5/QtCore (compatibility version 5.15.0, current version 5.15.16)
/System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
/System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics (compatibility version 64.0.0, current version 1889.2.7)
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 3208.0.0)
/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/AGL.framework/Versions/A/AGL (compatibility version 1.0.0, current version 1.0.0)
/usr/local/opt/libpng/lib/libpng16.16.dylib (compatibility version 64.0.0, current version 64.0.0)
/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.12)
/System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices (compatibility version 1.0.0, current version 65.0.0)
/usr/local/opt/md4c/lib/libmd4c.0.dylib (compatibility version 0.0.0, current version 0.5.2)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1800.105.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3208.0.0)
/System/Library/Frameworks/CoreText.framework/Versions/A/CoreText (compatibility version 1.0.0, current version 844.2.0)
/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
% objdump --macho --dylibs-used $(brew --prefix qt@6)/Frameworks/QtGui.framework/QtGui
/usr/local/opt/qt/Frameworks/QtGui.framework/QtGui:
/usr/local/opt/qt/lib/QtGui.framework/Versions/A/QtGui (compatibility version 6.0.0, current version 6.9.0)
/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit (compatibility version 45.0.0, current version 2575.30.19)
/System/Library/Frameworks/Carbon.framework/Versions/A/Carbon (compatibility version 2.0.0, current version 170.0.0)
/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3208.0.0)
/System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics (compatibility version 64.0.0, current version 1889.2.7)
/System/Library/Frameworks/CoreText.framework/Versions/A/CoreText (compatibility version 1.0.0, current version 844.2.0)
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 3208.0.0)
/System/Library/Frameworks/QuartzCore.framework/Versions/A/QuartzCore (compatibility version 1.2.0, current version 1.11.0)
/System/Library/Frameworks/Metal.framework/Versions/A/Metal (compatibility version 1.0.0, current version 367.6.0)
/usr/local/opt/glib/lib/libglib-2.0.0.dylib (compatibility version 8401.0.0, current version 8401.0.0)
@rpath/QtDBus.framework/Versions/A/QtDBus (compatibility version 6.0.0, current version 6.9.0)
/System/Library/Frameworks/AGL.framework/Versions/A/AGL (compatibility version 1.0.0, current version 1.0.0)
/usr/local/opt/libpng/lib/libpng16.16.dylib (compatibility version 64.0.0, current version 64.0.0)
/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.12)
/usr/local/opt/harfbuzz/lib/libharfbuzz.0.dylib (compatibility version 61100.0.0, current version 61100.0.0)
/usr/local/opt/md4c/lib/libmd4c.0.dylib (compatibility version 0.0.0, current version 0.5.2)
/usr/local/opt/freetype/lib/libfreetype.6.dylib (compatibility version 27.0.0, current version 27.2.0)
/usr/local/opt/glib/lib/libgthread-2.0.0.dylib (compatibility version 8401.0.0, current version 8401.0.0)
@rpath/QtCore.framework/Versions/A/QtCore (compatibility version 6.0.0, current version 6.9.0)
/System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
/System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/UniformTypeIdentifiers.framework/Versions/A/UniformTypeIdentifiers (compatibility version 1.0.0, current version 709.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1800.105.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)
/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices (compatibility version 1.0.0, current version 1226.0.0)
/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
```
2. Different directory layout:
```
% ls -l $(brew --prefix qt@5)/
total 544
drwxr-xr-x 79 hebasto admin 2528 13 Nov 06:22 Frameworks
-rw-r--r-- 1 hebasto admin 7533 16 Apr 09:09 INSTALL_RECEIPT.json
-rw-r--r-- 1 hebasto admin 22961 13 Nov 06:22 LICENSE.FDL
-rw-r--r-- 1 hebasto admin 36363 13 Nov 06:22 LICENSE.GPL3-EXCEPT
-rw-r--r-- 1 hebasto admin 15351 13 Nov 06:22 LICENSE.GPLv2
-rw-r--r-- 1 hebasto admin 35641 13 Nov 06:22 LICENSE.GPLv3
-rw-r--r-- 1 hebasto admin 26828 13 Nov 06:22 LICENSE.LGPLv21
-rw-r--r-- 1 hebasto admin 8174 13 Nov 06:22 LICENSE.LGPLv3
-rw-r--r-- 1 hebasto admin 106262 13 Nov 06:22 LICENSE.QT-LICENSE-AGREEMENT
-rw-r--r-- 1 hebasto admin 3842 13 Nov 06:22 README
drwxr-xr-x 57 hebasto admin 1824 16 Apr 09:09 bin
drwxr-xr-x 4 hebasto admin 128 13 Nov 06:22 doc
drwxr-xr-x 95 hebasto admin 3040 13 Nov 06:22 include
drwxr-xr-x 119 hebasto admin 3808 16 Apr 09:09 lib
drwxr-xr-x 8 hebasto admin 256 13 Nov 06:22 libexec
drwxr-xr-x 79 hebasto admin 2528 16 Apr 09:09 mkspecs
drwxr-xr-x 15 hebasto admin 480 13 Nov 06:22 phrasebooks
drwxr-xr-x 31 hebasto admin 992 13 Nov 06:22 plugins
drwxr-xr-x 28 hebasto admin 896 13 Nov 06:22 qml
-rw-r--r-- 1 hebasto admin 6952 16 Apr 09:09 sbom.spdx.json
drwxr-xr-x 3 hebasto admin 96 13 Nov 06:22 share
drwxr-xr-x 347 hebasto admin 11104 13 Nov 06:22 translations
% ls -l $(brew --prefix qt@6)/share/qt/
total 0
drwxr-xr-x 4 hebasto admin 128 30 Mar 09:49 doc
drwxr-xr-x 35 hebasto admin 1120 16 Apr 09:16 libexec
drwxr-xr-x 167 hebasto admin 5344 30 Mar 09:49 metatypes
drwxr-xr-x 70 hebasto admin 2240 16 Apr 09:16 mkspecs
drwxr-xr-x 178 hebasto admin 5696 30 Mar 09:49 modules
drwxr-xr-x 15 hebasto admin 480 30 Mar 09:49 phrasebooks
drwxr-xr-x 31 hebasto admin 992 30 Mar 09:49 plugins
drwxr-xr-x 34 hebasto admin 1088 30 Mar 09:49 qml
drwxr-xr-x 45 hebasto admin 1440 30 Mar 09:49 sbom
drwxr-xr-x 285 hebasto admin 9120 30 Mar 09:49 translations
```
This PR addresses both issues and additionally adds a `deploy` target to the native macOS CI job to prevent any similar recessions in the future.
Fixes https://github.com/bitcoin/bitcoin/issues/32267.
ACKs for top commit:
fanquake:
ACK 84de8c93e7d4979575161a2bb8f7eb64e1317b89
Tree-SHA512: 27a0eff3cd9317647529ff4571bd79c5dd8f007775b19415c8c27ca4912a60d85074c840cf0443be314d9a404f78bb015029d46dab18e292462249a5d90c6c47
Homebrew’s `qt@6` package places the `translations` and `plugins`
directories in the `share/qt` subdirectory.
This change updates the `macdeployqtplus` script accordingly.
Extend `getFrameworks()` with an optional `rpath` parameter to replace
`@rpath` in dependency paths. This fixes resolution of framework
dependencies when using Homebrew's `qt@6` package.
fa21f83d2983d97006ec1e3c47634dc0fe0349dc ci: Use G++ in valgrind tasks (MarcoFalke)
fabd05bf651138679f76728f974f141ac8ce99a9 refactor: Fix net_processing iwyu includes (MarcoFalke)
fa1622db208025e1744e78c4f5b135db11b293d4 refactor: Make node_id a const& in RemoveBlockRequest (MarcoFalke)
Pull request description:
Currently, `valgrind` is not usable on a default build with GCC. Specifically, `p2p_compactblocks.py --valgrind` gives a false-positive in `RemoveBlockRequest` when comparing `node_id` with `from_peer`. According to the upstream bug report, this happens because both symbols are on the stack and the compiler can more aggressively optimize the compare (order). See https://bugs.kde.org/show_bug.cgi?id=472329#c7
It is possible to work around this bug by pulling at least one value from the stack. For example, by making `from_peer` a `const` reference. Alternatively, by replacing `auto [node_id, list_it]` with `const auto& [node_id, list_it]`, which is done here.
I think this workaround is acceptable, because it does not look like valgrind can trivially fix this. The alternative would be to add a (temporary?) suppression.
Fixes https://github.com/bitcoin/bitcoin/issues/27741
Also, fix iwyu includes, while touching this module.
Also, switch the CI valgrind scripts to use G++.
ACKs for top commit:
achow101:
ACK fa21f83d2983d97006ec1e3c47634dc0fe0349dc
TheCharlatan:
ACK fa21f83d2983d97006ec1e3c47634dc0fe0349dc
darosior:
utACK fa21f83d2983d97006ec1e3c47634dc0fe0349dc
ryanofsky:
Code review ACK fa21f83d2983d97006ec1e3c47634dc0fe0349dc. Code changes all look good but I'm a little confused about purpose of the third commit, so left a question about that
Tree-SHA512: 7b92cdafd525a5ac53ae2c1a7a92e599bc9b5fd5d315a694b493cd5079ac323d884393b57aa18581b7789247a588c9a27d47698de25b340bc76fc9f1dd1850b4
When iterating over all fuzz input files in a folder, the order should
not matter.
However, shuffling may be useful to detect non-determinism.
Thus, shuffle in fuzz.cpp, when using neither libFuzzer, nor AFL.
Also, shuffle in the deterministic-fuzz-coverage tool, when using
libFuzzer.
f00345727b8d2bf73409db5cd342e476671e6425 doc: Update `dependencies.md` for Qt 6 (Hennadii Stepanov)
80b917991ed7ff931f0a9211cebf859f674776c4 build, msvc: Update `vcpkg.json` for Qt 6 (Hennadii Stepanov)
30dd1f1644e0441b5310f1eceecfd6a5abc45f68 ci: Update for Qt 6 (Hennadii Stepanov)
629d292f4d846978c682c5f497240c62d62f4bd1 test: Update sanitizer suppressions for Qt 6 (Hennadii Stepanov)
551e13abf82522bad7fdde4ff4bd15d2c8f88b23 guix: Adjust for Qt 6 (Hennadii Stepanov)
c3e9bd086c498e9f1cbdc505949cb17ac1b39f7e qt: Fix compiling for Windows (Hennadii Stepanov)
ab399c4db2e98aee8e01323c4c6cca48b520dc7f depends: Add `native_qt` package (Hennadii Stepanov)
248613eb3ee034bf143821a51635e697dc114e6c depends: Factor out Qt modules' details (Hennadii Stepanov)
0268f52a4cd2b7aa63934526437bcf6912e47d3c depends: Introduce customizable `$(package)_patches_path` variables (Hennadii Stepanov)
5e794e62024eef612e1fbb71c76ea54d17435c14 depends: Bump `qt` package up to 6.7.3 (Hennadii Stepanov)
6d4214925fadc36d26aa58903db5788c742e68c6 cmake: Require Qt 6 to build GUI (Hennadii Stepanov)
Pull request description:
The currently used Qt 5.15 is approaching [EOL](https://www.qt.io/blog/qt-5.15-extended-support-for-subscription-license-holders) and will reach it before the Bitcoin Core v30 release. The recent migration of the build system to CMake makes it possible to switch to Qt 6.
This PR updates the OS runtime compatibility requirements for the Bitcoin Core GUI as follows:
### 1. Linux
Starting with Qt 6.5.0, the `libxcb-cursor0` package is required to be installed at runtime.
### 2. Windows
Cross-compiling does not support LTO. We have to re-add it in a follow-up.
A new style plugin causes minor visual glitches, such as
which will be fixed in follow-ups.
### 3. macOS
`bitcoin-qt` now uses the [Metal](https://developer.apple.com/metal/) backend.
---
**IMPORTANT.** Don't forget to install [Ninja](https://ninja-build.org/).
---
For historical context, please refer to:
- https://github.com/bitcoin/bitcoin/issues/20627
- https://github.com/bitcoin/bitcoin/pull/24798
---
UPD 2024-10-09. Qt 6.8 has been [released](https://www.qt.io/blog/qt-6.8-released), but it has some [drawbacks](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402990346) for us. As a result, this PR will stick to Qt 6.7.
UPD 2025-03-18: [Standard support for Qt 5.15 will end after 26th of May 2025](https://www.qt.io/blog/extended-security-maintenance-for-qt-5.15-begins-may-2025)
ACKs for top commit:
laanwj:
re-ACK f00345727b8d2bf73409db5cd342e476671e6425
hodlinator:
re-ACK f00345727b8d2bf73409db5cd342e476671e6425
Tree-SHA512: 367f722e6c3ea4700b5395871c40b6df8c8062fdc822107090449ea4ae4ad2db75cc53a982a678f4c48ce8f9b2d43ed10e6d23b06165ab78713f161db712d895
1. Do not set `C{PLUS}_INCLUDE_PATH` variables
The build system for Qt 6 differs entirely from that of Qt 5. Building a
set of native Qt 6 tools now forms a separate step when cross-compiling.
Under these new circumstances, the `C{PLUS}_INCLUDE_PATH` environment
variables may alter the default include directories for both native and
cross compilers.
Previously, we explicitly unset these variables when invoking clang for
cross-compiling; however, that approach proved suboptimal (see #30451).
This change sets the native toolchain for dependencies explicitly,
rather than relying on the `C{PLUS}_INCLUDE_PATH` environment variables.
Additionally, it facilitates the transition towards using clang for
building native tools when cross-compiling for macOS.
2. Add `ninja` package.
3. Adjust allowed symbol lists.
fa513101212327f45965092652f6497aa28362ec contrib: Warn about using libFuzzer for coverage check (MarcoFalke)
fa17cdb191de71cdb4151408450aa9b3eaea6cb8 test: Avoid script check worker threads while fuzzing (MarcoFalke)
fa900bb2dce8ef3ee11d5980f008995d66877155 contrib: Only print fuzz output on failure (MarcoFalke)
fa82fe2c7364226a758c4a59e1a4a62e3ac4846b contrib: Use -Xdemangler=llvm-cxxfilt in deterministic-*-coverage (MarcoFalke)
fa7e93113052d09cc5ce4332d1c15904341bd132 contrib: Add optional parallelism to deterministic-fuzz-coverage (MarcoFalke)
Pull request description:
This should make the `partially_downloaded_block` fuzz target even more deterministic.
Follow-up to https://github.com/bitcoin/bitcoin/pull/31841. Tracking issue: https://github.com/bitcoin/bitcoin/issues/29018.
This bundles several changes:
* First, speed up the `deterministic-fuzz-coverage` helper by introducing parallelism.
* Then, a fix to remove spawned test threads or spawn them deterministically. (While testing this, high parallelism and thread contention may be needed)
### Testing
It can be tested via (setting 32 parallel threads):
```
cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../b-c-qa-assets/fuzz_corpora/ partially_downloaded_block 32
```
Locally, on a failure, the output would look like:
```diff
....
- 150| 0| m_worker_threads.emplace_back([this, n]() {
- 151| 0| util::ThreadRename(strprintf("scriptch.%i", n));
+ 150| 1| m_worker_threads.emplace_back([this, n]() {
+ 151| 1| util::ThreadRename(strprintf("scriptch.%i", n));
...
```
This excerpt likely indicates that the script threads were started after the fuzz init function returned.
Similarly, for the scheduler thread, it would look like:
```diff
...
227| 0| m_node.scheduler = std::make_unique<CScheduler>();
- 228| 1| m_node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { m_node.scheduler->serviceQueue(); });
+ 228| 0| m_node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { m_node.scheduler->serviceQueue(); });
229| 0| m_node.validation_signals =
...
```
ACKs for top commit:
Prabhat1308:
re-ACK [`fa51310`](fa51310121)
hodlinator:
re-ACK fa513101212327f45965092652f6497aa28362ec
janb84:
Re-ACK [fa51310](fa51310121)
Tree-SHA512: 1a935eb19da98c7c3810b8bcc5287e5649ffb55bf50ab78c414a424fef8e703839291bb24040a552c49274a4a0292910a00359bdff72fa29a4f53ad36d7a8720
6afffba34e086de4cf0bb86729e12d116c1dcc9b contrib: (asmap) add docs about encode and decode commands (jurraca)
67d5cc2a06e6c74e82221d35e2fc03ed6580b240 contrib: (asmap) add documentation on diff and diff-addrs commands (jurraca)
e047b1deca06fd8aafd4aa31d69fb70ef09a7995 contrib: (asmap) add diff-addrs example to README (jurraca)
Pull request description:
This README was a little sparse in my opinion, and was missing a mention of the `diff-addrs` command.
The README updates add background and examples for each command, split in two sections (encode/decode and diff/diff-addrs). This is intended to help people know how and when to run the commands available in the `asmap-tool.py` script.
However, I could use some confirmation on the behavior of the `--fill` flag. It's true that files generated with this flag set cannot be used to diff files after the fact, but i don't quite follow what the fill flag does to make that true. sipa could you maybe provide some insight?
ACKs for top commit:
fjahr:
re-ACK 6afffba34e086de4cf0bb86729e12d116c1dcc9b
brunoerg:
reACK 6afffba34e086de4cf0bb86729e12d116c1dcc9b
laanwj:
re-ACK 6afffba34e086de4cf0bb86729e12d116c1dcc9b
Tree-SHA512: 073e8d7255f7270aa2f5a070332872f5fa6fbe6532eee1f7e3e4158ac0125a49c155f4933bf00655ff3a89f666f3f3bea521e70c516ab09a448845016d2b880a
This makes it humanly possible to track progress as only "[N/M]"-lines are printed as long as we succeed.
Also, use char (a, b) to indicate run_id instead of u8 (0, 1).
Also, use emojis to indicate final success or error.
Co-Authored-By: Hodlinator <172445034+hodlinator@users.noreply.github.com>
fa7a40d952ad7588f45f6229aeae754b02fdfb55 contrib: Print deterministic-coverage runs (MarcoFalke)
fa751639fb667dc31a2220e33cbd134aa6b30741 contrib: Make deterministic-coverage error messages more readable (MarcoFalke)
Pull request description:
This is almost a "refactor" to tidy up the error messages. Apart from the messages, the behavior of the tools is identical.
This was requested in https://github.com/bitcoin/bitcoin/pull/31901#discussion_r1969502508.
Previously, the tool would abort the program early on some errors. Now, the tool propagates an `std::result::Result::Err` up to `main` via an early return. Getting rid of the aborts also allows to drop the `RUST_BACKTRACE` env setting.
ACKs for top commit:
hodlinator:
re-ACK fa7a40d952ad7588f45f6229aeae754b02fdfb55
janb84:
ACK [fa7a40d](fa7a40d952)
Tree-SHA512: 6c97861306e2fececa14b2d12deafb78995fc2bcf75e4e22773cb0ab4231de78834db9f1f89b30c49d77499433b1c16c1d90b97eb4069c81855bd2a7944b554f
Pass bitcoin binary command lines from test framework to signet/miner utility
using shell escaping so they are unambigous and don't get mangled if they
contain spaces.
This change is not needed for tests to pass currently, but is a useful change
to avoid CI failures in followup PR
https://github.com/bitcoin/bitcoin/pull/31375 and to avoid other bugs.
a24419f8bed5e1145ce171dbbdad957750585471 contrib: Fix `gen-bitcoin-conf.sh`. (David Gumberg)
Pull request description:
In #31118, the format of bitcoind's `--help` output changed slightly in a way that breaks `gen-bitcoin-conf.sh`, modify the script to accommodate the new format, by starting after the line that says "Options:" and stripping the `-help` options and descriptions from the script output.
Before this PR, all options above `-help` were excluded from the example bitcoin.conf.
ACKs for top commit:
mabu44:
Tested ACK a24419f8bed5e1145ce171dbbdad957750585471
glozow:
ACK a24419f8bed5e1145ce171dbbdad957750585471
rkrux:
tACK a24419f8bed5e1145ce171dbbdad957750585471
BrandonOdiwuor:
crACK a24419f8bed5e1145ce171dbbdad957750585471
Tree-SHA512: 2ef697166d0b37b61ec1a20e357b91d611c932a0e453c4669f74ab69e6310ea1776dce09c1b77e82746072265763cb0c750e6df4c8b4a7d39068fc03f97b221b
fa99c3b544b631cfe34d52fb5e71636aedb1b423 test: Exclude SeedStartup from coverage counts (MarcoFalke)
fa579d663d716c967ccd45d67b46e779e2fa0b48 contrib: Add deterministic-unittest-coverage (MarcoFalke)
fa3940b1cbc94c8ccfde36be1db1adca04fbcaa6 contrib: deterministic-fuzz-coverage fixups (MarcoFalke)
faf905b9b694313bed4531d1299568a101f33fb8 doc: Remove unused -fPIC (MarcoFalke)
fa1e0a72281fde13d704c7766d4d704e009274da gitignore: target/ (MarcoFalke)
Pull request description:
The `contrib/devtools/test_deterministic_coverage.sh` script is problematic:
* It is written in bash. This can lead to issues when running with the ancient bash version shipped by macOS by default, or can lead to other compatibility issues, such as https://github.com/bitcoin/bitcoin/pull/31588#discussion_r1946784827. Also, pipefail isn't set, so IO errors may be silently ignored.
* It is based on gcov. This can lead to issues, such as https://github.com/bitcoin/bitcoin/pull/31588#pullrequestreview-2602169248 (possibly due to prefix-map), or https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2646395385 (gcovr processing error), or https://github.com/bitcoin/bitcoin/pull/31588#pullrequestreview-2605954001 (gcovr assertion error).
* The script is severely outdated, with the last update to `NON_DETERMINISTIC_TESTS` being in the prior decade.
Instead of patching around all issues one-by-one, just provide a fresh rewrite, based on the recently added `deterministic-fuzz-coverage` tool based on clang, llvm-cov, and llvm-profdata. (Initial feedback indicates that this is a more promising attempt: https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2649356408 and https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2649354598).
The new tool also sets `RANDOM_CTX_SEED=21` as suggested by hodlinator in https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2650784726.
ACKs for top commit:
Prabhat1308:
Concept ACK [`fa99c3b`](fa99c3b544)
hodlinator:
re-ACK fa99c3b544b631cfe34d52fb5e71636aedb1b423
brunoerg:
light ACK fa99c3b544b631cfe34d52fb5e71636aedb1b423
dergoegge:
tACK fa99c3b544b631cfe34d52fb5e71636aedb1b423
janb84:
Concept ACK [fa99c3b](fa99c3b544)
Tree-SHA512: 491d5e6413d929395a5c7caea54817bdc1a0e00562c9728a374d4e92f2e2017dba4a770ecdb2e7317e049df9fdeb390d83c90dff9aa5709f97aa3f6a0e70cdb4
In #31118, the format of bitcoind's `--help` output changed slightly in
a way that breaks `gen-bitcoin-conf.sh`, modify the script to accomodate
the new format, by starting after the line that says "Options:" and
strip the `-help` option and its description from the output.
568fcdddaec2cc8decba5a098257f31729cc1caa scripted-diff: Adjust documentation per top-level target output location (Hennadii Stepanov)
026bb226e96919603af829d0b677779a234a0f6e cmake: Set top-level target output locations (Hennadii Stepanov)
Pull request description:
This PR sets the target output locations to the `bin` and `lib` subdirectories within the build tree, creating a directory structure that mirrors that of the installed targets.
This approach is widely adopted by the large projects, such as [LLVM](e146c1867e/lldb/cmake/modules/LLDBStandalone.cmake (L128-L130)):
```cmake
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
```
The `libsecp256k1` project has also recently [adopted](https://github.com/bitcoin-core/secp256k1/pull/1553) this approach.
With this PR, all binaries are conveniently located. For example, run:
```
$ ./build/bin/fuzz
```
instead of:
```
$ ./build/src/test/fuzz/fuzz
```
On Windows, all required DLLs are now located in the same directory as the executables, allowing to run `bitcoin-chainstate.exe` (which loads `bitcoinkernel.dll`) without the need to copy DLLs or modify the `PATH` variable.
The idea was briefly discussed among the build team during the recent CoreDev meeting.
---
**Warning**: This PR changes build locations of newly built executables like `bitcoind` and `test_bitcoin` from `src/` to `bin/` without deleting previously built executables. A clean build is recommended to avoid accidentally running old binaries.
ACKs for top commit:
theStack:
Light re-ACK 568fcdddaec2cc8decba5a098257f31729cc1caa
ryanofsky:
Code review ACK 568fcdddaec2cc8decba5a098257f31729cc1caa. Only change since last review was rebasing. I'm ok with this PR in its current form if other developers are happy with it. I just personally think it is inappropriate to \*silently\* break an everyday developer workflow like `git pull; make bitcoind`. I wouldn't have a problem with this PR if it triggered an explicit error, or if the problem was limited to less common workflows like changing cmake options in an existing build.
TheCharlatan:
Re-ACK 568fcdddaec2cc8decba5a098257f31729cc1caa
theuni:
ACK 568fcdddaec2cc8decba5a098257f31729cc1caa
Tree-SHA512: 1aa5ecd3cd49bd82f1dcc96c8e171d2d19c58aec8dade4bc329df89311f9e50cbf6cf021d004c58a0e1016c375b0fa348ccd52761bcdd179c2d1e61c105e3b9f
We shouldn't have | at the end of the last clause, as this will make it match
the empty string too (so effectively everything starting with Satoshi: matches).
While doing this, put the | at the beginning of every line of regex rather than
the end, to make it easier to update in the future without accidentally running
into this problem again.
