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.
e181bda061ca63021511be6e286fdf6a5818df49 guix: Apply all codesignatures to Windows binaries (Ava Chow)
aafbd23fd97ac242f7f83e5f0fff20044176e126 guix: Apply codesignatures to all MacOS binaries (Ava Chow)
3656b828dc2204418974e94928cc8d915b10ed95 contrib: Sign all Windows binaries too (Ava Chow)
31d325464d0cf2d06888e0c543ae26a944f2ec6b contrib: Sign and notarize all MacOS binaries (Ava Chow)
710d5b5149d0bc36d2643281d81f8f9b0c51b480 guix: Update signapple (Ava Chow)
e8b3c44da6e060464970717bbd0a5bf84867b82c build: Include all Windows binaries for codesigning (Ava Chow)
dd4ec840eeb468e94cfc9e3c72cfbfd6704dc0da build: Include all MacOS binaries for codesigning (Ava Chow)
4e5c9ceb9dd5a6ad8eea689d916a632e4d482812 guix: Rename Windows unsigned binaries to unsigned.zip (Ava Chow)
d9d49cd533bd430776c0cbe2fd666ffec3e6637b guix: Rename MacOS binaries to unsigned.tar.gz (Ava Chow)
c214e5268fa9322a83cbba6d47d33f830efdd89e guix: Rename unsigned.tar.gz to codesigning.tar.gz (Ava Chow)
Pull request description:
I have updated signapple to notarize MacOS app bundles without adding any additional dependencies. Further, it can also sign and apply detached signatures to standalone binaries.
As such, we can use signapple to perform the notarization and stapling steps so that MacOS will run the app bundle after it is installed. `detached-sig-create.sh` is updated to have a notarization step and to download the ticket which will be included in the detached signatures. The workflow is largely unchanged for the MacOS codesigners except for the additional requirement of having an App Store Connect API key and Team UUID, instructions for which can be found at https://github.com/achow101/signapple/blob/master/docs/notarization.md. For guix builders, the workflow is unchanged.
Additionally, the standalone binaries packaged in the MacOS `.tar.gz` and Windows `.zip` will now be codesigned. `detached-sig-create.sh` was updated to handle these, so the workflow for both MacOS and Windows codesigners remains unchanged. For guix builders, the workflow is also unchanged.
Because those binaries will how have codesigned and unsigned versions, the build command is modified to output `-unsigned.{tar.gz,zip}` archives containing the binaries. Since this happens to conflict with the tarball used for codesigning, the codesigning tarball was renamed to `-codesigning.tar.gz`. Both MacOS and Windows codesigners will need to adjust their workflows to account for the new name.
Fixes#15774 and #29749
ACKs for top commit:
Sjors:
Tested ACK e181bda061ca63021511be6e286fdf6a5818df49
davidgumberg:
Tested ACK e181bda061.
pinheadmz:
tested ACK e181bda061ca63021511be6e286fdf6a5818df49
Tree-SHA512: ce0e2bf38e1748cdaa0d13be6f61c3289cd09cfb7d071a68b0b13d2802b3936c9112eda6e4c7b29c535c0995d56b14871442589cdcea2e7707e35c1b278b9263
* Name the fuzz_corpora dir after its real name.
* Add missing cargo lock file.
* Use git instead of diff command to increase compatibility
* Use --help instead of --version to increase compatibility
* Use assert consistently for unexpected errors.
* Remove redundant Stdio::from.
* Fix typos.
The utxo snapshot metadata doesn't seem to contain any block height as per the
CPP code and no such value is read few lines down by the tool code as well.
Related CPP code: bitcoin/bitcoin/blob/28.x/src/node/utxo_snapshot.h#L60-L66
Running the `dumptxoutset` command without a `type` parameter leads
to the following error. Update the tool documentation to make it
easier to follow.
`Invalid snapshot type "" specified. Please specify "rollback" or "latest"`
This change:
1. Collects build artifacts in dedicated locations.
2. Allows running bitcoin-chainstate.exe with bitcoinkernel.dll directly
from the build tree on Windows.
63a8791e15c3ffb44b84ab3e85db62d7997d25fd contrib: fix BUILDDIR in gen-bitcoin-conf script and gen-manpages.py (jurraca)
Pull request description:
The `gen-bitcoin-conf.sh` and `gen-manpages.py` scripts assume a top level `src/` build dir, but in-tree builds are no longer allowed, nor recommended in the build steps. If a user builds `bitcoind` as recommended, these scripts fail. To fix it, we update the `BUILDDIR` env var and update the README accordingly.
Follows up on initial work and discussion in #31332 .
ACKs for top commit:
fjahr:
Code review ACK 63a8791e15c3ffb44b84ab3e85db62d7997d25fd
achow101:
ACK 63a8791e15c3ffb44b84ab3e85db62d7997d25fd
Tree-SHA512: cf4d5b0d2e8b1f5db759bec01e131d8a0c511a2fd183389d2a0488d5fe4a906db2579d944f408b5c966f619edc6b2534023c3521f1fa5f8edd0216d29f3e48db
Signapple has been updated to sign individual binaries, and notarize app
bundles and binaries. When codesigning, all individual binaries will be
codesigned, and both the app bundle and individual binaries will be
notarized.
0c1b29a05777256c5ee686fff60f281dfeae289c ci: use GCC 13 for some jobs (fanquake)
cbc65b3ad5ad573844f9841199e1b0817f6c648a guix: use GCC 13.3.0 for base toolchain. (fanquake)
Pull request description:
Switch release builds to using GCC 13.3.0: https://gcc.gnu.org/gcc-13/, which landed in Guix in: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=750148ce1ea6c65a7c14424546db0078161f7e17.
Does not solve the cross-arch non-determinism for `powerpc64le-linux-gnu` builds.
ACKs for top commit:
achow101:
ACK 0c1b29a05777256c5ee686fff60f281dfeae289c
hebasto:
ACK 0c1b29a05777256c5ee686fff60f281dfeae289c.
TheCharlatan:
Re-ACK 0c1b29a05777
Tree-SHA512: eb3f091278d371166eb1df4718b6d0d68b09db65291d563dddd581964f2b488f901e4ba43831a699e2d0fd053d6e9038a307cbea78d5597da77699c34b440ea6
9cf746d6631739df9c9f80accd5812b319efcfec cmake: add optional source files to crc32c directly (Daniel Pfeifer)
9c7823c5b531ac1bbe5bdb9f2731bfae06cf695a cmake: add optional source files to bitcoin_crypto directly (Daniel Pfeifer)
Pull request description:
Avoid having many static libraries by adding the optional sources to the target `bitcoin_crypto` directly.
Set the necessary compile options at the source file level, rather than the target level.
fixes: #31697
ACKs for top commit:
s373nZ:
ACK 9cf746d6631739df9c9f80accd5812b319efcfec
hebasto:
re-ACK 9cf746d6631739df9c9f80accd5812b319efcfec.
TheCharlatan:
ACK 9cf746d6631739df9c9f80accd5812b319efcfec
Tree-SHA512: 04b468ccbd284d63fc83b382177bb8183b325369835c3b92e555e159955c73d71712a63a2e556f8da68a1232ac07d3845e11f1057c50666843db91db98fca979
fa3e409c9a084112fc2644a2bba9aa196bdb229d contrib: Add deterministic-fuzz-coverage (MarcoFalke)
Pull request description:
The goal of this script is to detect and debug the remaining fuzz determinism and stability issues (https://github.com/bitcoin/bitcoin/issues/29018).
ACKs for top commit:
marcofleon:
Tested ACK fa3e409c9a084112fc2644a2bba9aa196bdb229d
brunoerg:
tested ACK fa3e409c9a084112fc2644a2bba9aa196bdb229d
Tree-SHA512: f336537d64188d6bc3c53880f4552a09cc498841c539cb7b4f14e622c9542531b970c1a6910981f7506e7bf659d2ce83471d58f5f51b0a411868f4c11eaf6b2a
4080b66cbec2b6fc2fcfd7356941236f65d508e3 test: add test for utxo-to-sqlite conversion script (Sebastian Falbesoner)
ec99ed738083fc1ad4c9d85095e26e7e58372217 contrib: add tool to convert compact-serialized UTXO set to SQLite database (Sebastian Falbesoner)
Pull request description:
## Problem description
There is demand from users to get the UTXO set in form of a SQLite database (#24628). Bitcoin Core currently only supports dumping the UTXO set in a binary _compact-serialized_ format, which was crafted specifically for AssumeUTXO snapshots (see PR #16899), with the primary goal of being as compact as possible. Previous PRs tried to extend the `dumptxoutset` RPC with new formats, either in human-readable form (e.g. #18689, #24202), or most recently, directly as SQLite database (#24952). Both are not optimal: due to the huge size of the ever-growing UTXO set with already more than 80 million entries on mainnet, human-readable formats are practically useless, and very likely one of the first steps would be to put them in some form of database anyway. Directly adding SQLite3 dumping support on the other hand introduces an additional dependency to the non-wallet part of bitcoind and the risk of increased maintenance burden (see e.g. https://github.com/bitcoin/bitcoin/pull/24952#issuecomment-1163551060, https://github.com/bitcoin/bitcoin/issues/24628#issuecomment-1108469715).
## Proposed solution
This PR follows the "external tooling" route by adding a simple Python script for achieving the same goal in a two-step process (first create compact-serialized UTXO set via `dumptxoutset`, then convert it to SQLite via the new script). Executive summary:
- single file, no extra dependencies (sqlite3 is included in Python's standard library [1])
- ~150 LOC, mostly deserialization/decompression routines ported from the Core codebase and (probably the most difficult part) a little elliptic curve / finite field math to decompress pubkeys (essentialy solving the secp256k1 curve equation y^2 = x^3 + 7 for y given x, respecting the proper polarity as indicated by the compression tag)
- creates a database with only one table `utxos` with the following schema:
```(txid TEXT, vout INT, value INT, coinbase INT, height INT, scriptpubkey TEXT)```
- the resulting file has roughly 2x the size of the compact-serialized UTXO set (this is mostly due to encoding txids and scriptpubkeys as hex-strings rather than bytes)
[1] note that there are some rare cases of operating systems like FreeBSD though, where the sqlite3 module has to installed explicitly (see #26819)
A functional test is also added that creates UTXO set entries with various output script types (standard and also non-standard, for e.g. large scripts) and verifies that the UTXO sets of both formats match by comparing corresponding MuHashes. One MuHash is supplied by the bitcoind instance via `gettxoutsetinfo muhash`, the other is calculated in the test by reading back the created SQLite database entries and hashing them with the test framework's `MuHash3072` module.
## Manual test instructions
I'd suggest to do manual tests also by comparing MuHashes. For that, I've written a go tool some time ago which would calculate the MuHash of a sqlite database in the created format (I've tried to do a similar tool in Python, but it's painfully slow).
```
$ [run bitcoind instance with -coinstatsindex]
$ ./src/bitcoin-cli dumptxoutset ~/utxos.dat
$ ./src/bitcoin-cli gettxoutsetinfo muhash <block height returned in previous call>
(outputs MuHash calculated from node)
$ ./contrib/utxo-tools/utxo_to_sqlite.py ~/utxos.dat ~/utxos.sqlite
$ git clone https://github.com/theStack/utxo_dump_tools
$ cd utxo_dump_tools/calc_utxo_hash
$ go run calc_utxo_hash.go ~/utxos.sqlite
(outputs MuHash calculated from the SQLite UTXO set)
=> verify that both MuHashes are equal
```
For a demonstration what can be done with the resulting database, see https://github.com/bitcoin/bitcoin/pull/24952#pullrequestreview-956290477 for some example queries. Thanks go to LarryRuane who gave me to the idea of rewriting this script in Python and adding it to `contrib`.
ACKs for top commit:
ajtowns:
ACK 4080b66cbec2b6fc2fcfd7356941236f65d508e3 - light review
achow101:
ACK 4080b66cbec2b6fc2fcfd7356941236f65d508e3
romanz:
tACK 4080b66cbe on signet (using [calc_utxo_hash](8981aa3e85/calc_utxo_hash/calc_utxo_hash.go)):
tdb3:
ACK 4080b66cbec2b6fc2fcfd7356941236f65d508e3
Tree-SHA512: be8aa0369a28c8421a3ccdf1402e106563dd07c082269707311ca584d1c4c8c7b97d48c4fcd344696a36e7ab8cdb64a1d0ef9a192a15cff6d470baf21e46ee7b
The tarballs used for codesigning are more than merely unsigned, they
also contain scripts and other data for codesigning. Rename them to
codesigning.tar.gz to distinguish from tarballs containing actually just
the unsigned binaries.
These scripts are becoming more of nuisance, than a value-add;
particularly since we've been building releases using Guix. Adding new
(release bin) tests can be harder, because it requires constructing a
failing test, which is becoming less easy e.g trying to disable a
feature or protection that has been built into the compiler/toolchain by
default.
In the pre-Guix days, these were valuable to sanity-check the environment,
because we were pulling that pre-built from Ubuntu, with little control.
At this point, it's less clear what these scripts are (sanity) checking.
Note that these also weren't completely ported to CMake (#31698), see
also #31715 which contains other fixes that would be needed for these
test-tests, to accomodate future changes.
the cmake build steps suggest a build/ directory, which breaks these
scripts. Additionally, in-tree builds are no longer allowed, so it makes
sense to update the code and the README accordingly.
