0972f5504021b482b27523fd3bcb8036cf6b439c from #33229 broke manpage
generation, because the assumption that the last word in the line
containing the version number, was the version number, no-longer holds
for some binaries. i.e bitcoind.
Github-Pull: #33996
Rebased-From: e9536faaee2bdb53807aa7c48aafa269d18f19d3
Starting with Python 3.11, Pythons gzip might delegate to zlib.
Depending on the OS, i.e Ubuntu vs Fedora, the underlying zlib
implementation might differ, resulting in different output.
For now, or until a better solution exists, disable compression. This
results in the SDK increasing in size to ~157mb. Which is not
unreasonable, to regain determinism (and would be significantly worse
without the previous commit).
See: https://docs.python.org/3/library/gzip.html#gzip.compress
Co-authored-by: stickies-v <stickies-v@protonmail.com>
Github-Pull: #32009
Rebased-From: c1213a35abed01a97a9c52954919158f91f974d2
Only include what we really need. Skip 100s of mb of manpages,
swiftmodules, modulemaps.
Note that System/Library is only needed for the Qt build.
Github-Pull: #32009
Rebased-From: a33d03454508187abed764e55351ffcececc4c6e
QT translations are optional, but the script would error when
'translations_dir' falls back to its default value NULL.
This PR fixes it by moving the set-up of QT translations under
the check for 'translations_dir' presence.
Github-Pull: #33482
Rebased-From: 7b5261f7ef3d88361204c40eb10c0d9dc44f5ed7
Modern Ubuntu isn't shipping with this library installed by default.
Staticly link it to remove the need for end-users to install it.
Closes#33432.
Github-Pull: #33434
Rebased-From: eca50854e1cb04e20478bd3df4762e18520a3611
a2a35b58cb957b7fba113eb4cf933d5e7b9bf8ef doc: fix LIBRARY_PATH comment (fanquake)
Pull request description:
Now that we build capnp, qt isn't the only native package.
ACKs for top commit:
l0rinc:
code review ACK a2a35b58cb957b7fba113eb4cf933d5e7b9bf8ef
willcl-ark:
ACK a2a35b58cb957b7fba113eb4cf933d5e7b9bf8ef
Tree-SHA512: 7b4a54b53b4c673e31dea011e569c6ef9e7238a56a7f09c7f5a7bc521fb0ffdda3794f2b65764908005333a30f7c8c7b4d6b45d985bc92a70dc2daf8c3d1ff3d
The Script errors were last touched in 2020. This value was calculated after that
in 2022 (commit 4b7aec2951fe4595946cdc804b0dec1921d79d05). The previous commit
made the size of the largest reject reason string 4 characters smaller ("mandatory"
became "block"), so adapt the constant.
Using "block" or "mempool" as the prefix in place of "mandatory" or "non-mandatory" is clearer
to a user. "non-mandatory" was renamed into "mempool" as part of #33050. This takes care of the
other half of this renaming as a scripted diff.
-BEGIN VERIFY SCRIPT-
sed -i 's/mandatory-script-verify/block-script-verify/g' $(git grep -l mandatory-script-verify)
-END VERIFY SCRIPT-
Fixes:
```bash
/distsrc-base/distsrc-d3b8a54a8120-x86_64-linux-gnu/build/bin/bitcoind: symbol __divmodti4 from unsupported version GCC_7.0.0(14)
/distsrc-base/distsrc-d3b8a54a8120-x86_64-linux-gnu/build/bin/bitcoind: failed IMPORTED_SYMBOLS
```
which is occuring after #32750. I can't see any supported distro that is
shipping a new enough glibc (2.31), but a GCC older than 7.0.
f49840dd902cd9b14b6aadb431b16a4aeb719c3f doc: Fix typo in files.md (Ryan Ofsky)
f5cf0b1ccc8fd426135809a8a4becdae2d797bb5 bitcoin wrapper: improve help output (Ryan Ofsky)
c810b168b89dc07017e9feaec1a8746a449a60b1 doc: Add description of installed files to files.md (Ryan Ofsky)
94ffd01a0294afbe045f1b17a77e4a3caf21e674 doc: Add release notes describing libexec/ binaries (Ryan Ofsky)
cd97905ebc564b8b095099a28d1d5437951927c4 cmake: Move internal binaries from bin/ to libexec/ (Ryan Ofsky)
Pull request description:
This change moves binaries that are not typically invoked directly by users from the `bin/` directory to the `libexec/` directory in CMake installs and binary releases. The goal of the PR is to introduce a distinction between internal and external binaries so starting with #31802, we can use IPC to implement features in new binaries without adding those binaries to the CLI. The change also helps reduce clutter in `bin/`, making it easier for users to identify useful tools to run. Summary of changes:
- For **source builds** (i.e. developer builds) — There are no changes.
- For **source installs** (i.e. `cmake --install` result) — `test_bitcoin`, `test_bitcoin-qt`, and `bench_bitcoin` are installed in `${CMAKE_PREFIX_PATH}/libexec` instead of `${CMAKE_PREFIX_PATH}/bin`, so they are no longer on the system `PATH`. However, they can still be invoked from the `libexec/` directory, or from the CLI as `bitcoin test`, `bitcoin test-gui`, and `bitcoin bench`, respectively.
- For **binary releases** — Since `test_bitcoin` is the only test binary enabled in releases, the only change is moving `test_bitcoin` from `bin/` to `libexec/`.
<details><summary>Details</summary>
<p>
The table below shows the install location of each binary after this change, and the availability of each binary.
| Binary | Location | Availability | Change |
|----------------------|--------------|----------------------|-------------------------------|
| `bitcoin` | `bin/` | 📦 Binary release (since #31375) | Unchanged |
| `bitcoin-cli` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoind` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-qt` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-tx` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-util` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-wallet` | `bin/` | 📦 Binary release | Unchanged |
| `bench_bitcoin` | `libexec/` | 🛠 Source build only | Moved from `bin/` |
| `bitcoin-chainstate` | `libexec/` | 🛠 Source build only | Newly installed (was built) |
| `bitcoin-gui` | `libexec/` | 🛠 Source build only (until #31802) | Moved from `bin/` |
| `bitcoin-node` | `libexec/` | 🛠 Source build only (until #31802) | Moved from `bin/` |
| `test_bitcoin` | `libexec/` | 📦 Binary release | Moved from `bin/` |
| `test_bitcoin-qt` | `libexec/` | 🛠 Source build only | Moved from `bin/` |
</p>
</details>
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
ACKs for top commit:
l0rinc:
re-ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
Sjors:
re-ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
achow101:
ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
janb84:
re ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
BrandonOdiwuor:
Tested ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
hodlinator:
re-ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
willcl-ark:
utACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
Tree-SHA512: 858a2e1a53db11ee3c5c759bfdeea566f242b9ce5e8a898fa435222e41662b8184577c0dc2c4c058294b4de41d8cb3ba3e5d24c748c280efa4a3f84e3ec4344d
PermissionsStartOnly is deprecated [1]. This removes the directives
and instead we prefixes the value of the ExecStartPre directive with
'!', which means the executable, 'chgrp' in this case, is run with
full privileges and able to change the group of /etc/bitcoin.
1: 60b45a80c1/NEWS (L2434)
Setting Group=bitcoin is redundant. It is typically the default group
of the user and if not explicitly specified, systemd will run the
service with the default group of the user.
1bed0f734b3f2dd876193b5cad303bfab1d250d5 guix: warn SOURCE_DATE_EPOCH set in guix-codesign (will)
Pull request description:
#32678 added a sanity check for this environment variable when running `guix-build` but missed that `guix-codesign` also relies on `SOURCE_DATE_EPOCH`, which can result in non-determinism in the codesigning step: https://github.com/bitcoin-core/guix.sigs/pull/1720#issuecomment-3124332676
To avoid repeating the logic move common functionality into the prelude and call the function in both guix actions.
ACKs for top commit:
fanquake:
ACK 1bed0f734b3f2dd876193b5cad303bfab1d250d5
Tree-SHA512: ad3de8ab06e7f4ffcee5c02e8185b20879d63a02a614a706ea54da5087cca4ba75817ca1aa95301572c34723317fcc44e4478261ac73dd223ee9fa827e6b35b3
The BPF code was incorrectly passing pointer variables by value to
bpf_usdt_readarg(), causing the function to fail silently and resulting
in transaction hashes and reason strings displaying as zeros or garbage.
This fix adds the missing reference operator (&) when passing pointer
variables to bpf_usdt_readarg(), allowing the function to properly
write the pointer values and enabling correct display of transaction
hashes and removal/rejection reasons.
Fixes the regression introduced in ec47ba349d where bpf_usdt_readarg_p
was replaced with bpf_usdt_readarg but the calling convention wasn't
properly updated for pointer arguments.
Currently there is a warning for this in guix-build, but we also need
one in guix-codesign, otherwise the codesigned hashes are not
reproducible.
Move common functionality into prelude and call the function in both
guix actions.
Since the previous commit, CBlockHeader/CBlock object calls to the
methods `.rehash()` and `.calc_sha256()` are effectively no-ops
if the returned value is not used, so we can just remove them.
4ef6253017672a74c584e6e9b449ffff79f67273 test: avoid unneeded (w)txid hex -> integer conversions (Sebastian Falbesoner)
472f3770aec89bf0783609094108ba277a46d339 scripted-diff: test: rename CTransaction `.getwtxid()` -> `wtxid_hex` for consistency (Sebastian Falbesoner)
81af4334e8f903601dac503426eaccc1d3527f9c test: rename CTransaction `.sha256` -> `.txid_int` for consistency (Sebastian Falbesoner)
ce83924237127fe6d32b63c362b57789e4628954 test: rename CTransaction `.rehash()`/`.hash` -> `.txid_hex` for consistency (Sebastian Falbesoner)
e9cdaefb0a80988cfba2dc5ddd96b84a204d5519 test: introduce and use CTransaction `.wtxid_int` property (Sebastian Falbesoner)
9b3dce24a3336a02563412b541a3fb2003e92506 test: remove bare CTransaction `.rehash()`/`.calc_sha256()` calls (Sebastian Falbesoner)
a2724e3ea392cdbd5526735516862b17bf687624 test: remove txid caching in CTransaction class (Sebastian Falbesoner)
Pull request description:
In the functional test framework, determining a (w)txid for a `CTransaction` instance is currently rather confusing and footgunny due to inconsistent naming/interfaces (see table below) and statefulness involved. This PR aims to improve that by:
* removing the (w)txid caching mechanism, in order to avoid the need to call additional rehashing functions (`.rehash()`/`.calculate_sha256()`, see first two commits and https://github.com/bitcoin/bitcoin/pull/32050#discussion_r1993286997). This change in theory decreases the performance, as the involved serialization and hashing involved might be called more often than previously, but I couldn't find a functional test where this leads to a measurable run-time increase on my machine.
* introduce consistent naming that shows the type of the returned txid, i.e. hex string vs. test-framework-internal representation [currently integers] (see remaining commits)
Summary table showing (w)txid determaination before/after this PR:
| Task | master | PR |
|:-----------------------|:-----------------------|:-------------|
| get TXID (hex string) | `.rehash()` / `.hash`[1] | `.txid_hex` |
| get TXID (integer) | `.sha256`[1] | `.txid_int` |
| get WTXID (hex string) | `.getwtxid()` | `.wtxid_hex` |
| get WTXID (integer) | `.calc_sha256(True)` | `.wtxid_int` |
Unfortunately, most renames can't be done with a scripted-diff, as the property names (`.hash`, `.sha256`) are also used for blocks and other message types. The PR is rather invasive and touches a lot of files, but I think it's worth to do it, also to make life easier for new contributors. Future tasks like e.g. doing the same overhaul for block (header) objects or getting rid of the integer representation (see https://github.com/bitcoin/bitcoin/pull/32050) become easier should become easier after this one.
[1] = returned value might be out-of-date, if rehashing function wasn't called after modification
ACKs for top commit:
maflcko:
re-ACK 4ef6253017672a74c584e6e9b449ffff79f67273 🏈
achow101:
ACK 4ef6253017672a74c584e6e9b449ffff79f67273
marcofleon:
code review ACK 4ef6253017672a74c584e6e9b449ffff79f67273
Tree-SHA512: 4b472c31d169966b6f6878911a8404d25bf3e503b6e8ef30f36a7415d21ad4bc1265083af2d3ead6edfcd9fac9ccb0a8be57e1b0739ad431b836413070d7d583
Some of the primary changes are:
- lief.EXE_FORMATS became lief.Binary.FORMATS IN 0.14.0
- 494f116c6b/doc/sphinx/changelog.rst (L702)
- lief.ARCHITECTURES became lief.Header.ARCHITECTURES in 0.16.0
- 494f116c6b/doc/sphinx/changelog.rst (L226C18-L227C18)
- lief.ELF.ARCH.x86_64 became lief.ELF.ARCH.X86_64
This commit includes a workaround for the bug fixed in
https://github.com/lief-project/LIEF/pull/1218, but the workaround can
be kept, since it makes `has_nx` checks stricter by enforcing both heap
and stack are non-executable.
This change also requires a patch to partially revert a commit to LIEF
(f23ced2f4f)
which broke compatibility with versions of scikit-build-core <= 0.10.x.
This patch can be dropped once the guix time machine advances to or
beyond 35c5f07e96,
which bumps the scikit-build-core version in guix from 0.9.3 to 0.10.7.
Co-authored-by: willcl-ark <will8clark@gmail.com>
Co-authored-by: fanquake <fanquake@gmail.com>
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Since the previous commit, CTransaction object calls to the
methods `.rehash()` and `.calc_sha256()` are effectively no-ops
if the returned value is not used, so we can just remove them.
Current behaviour will by-default use SOURCE_DATE_EPOCH from the
environment without warning. This breaks the default reproducibility
from a guix build.
Warn when and exit when this variable is set, and
FORCE_SOURCE_DATE_EPOCH is unset.
6ee32aaaca4a46d42594bc16479868c76cc67fd8 test: signet tool genpsbt and solvepsbt commands (Sjors Provoost)
0a99d99fe4cb8432945d37c8e5d221715c538e4f signet: miner skips PSBT step for OP_TRUE (Sjors Provoost)
cdfb70e5a6a9235ea18dd2cb8148b757902dc08e signet: split decode_psbt miner helper (Sjors Provoost)
Pull request description:
[BIP325](https://github.com/bitcoin/bips/blob/master/bip-0325.mediawiki) mentions the following rule:
> In the special case where an empty solution is valid (ie scriptSig and scriptWitness are both empty) this additional commitment can optionally be left out. This special case is to allow non-signet-aware block generation code to be used to test a custom signet chain where the challenge is trivially true.
Such a signet can be created using e.g. `-signetchallenge=51` (`OP_TRUE`). However `contrib/signet/miner` won't omit the commitment.
This PR improves the miner by skipping the PSBT for known trivial scripts (just `OP_TRUE` and trivial pushes for now). This prevents it from appending the 4 byte signet header to the witness commitment, as allowed by the above rule.
---
Previously the script would fail with `PSBT signing failed`, making it difficult to mine. This is no longer the case.
ACKs for top commit:
achow101:
ACK 6ee32aaaca4a46d42594bc16479868c76cc67fd8
theStack:
re-ACK 6ee32aaaca4a46d42594bc16479868c76cc67fd8
danielabrozzoni:
ACK 6ee32aaaca4a46d42594bc16479868c76cc67fd8
Tree-SHA512: e47fbf471f2909286a6c1c073799ea388b9c19551afcce96cf9af45cc48d25c02f1e48e08861a88b604361e2c107a759d5baf393da8a37360de419f31651758a
This change moves binaries that are not typically invoked directly by users
from the `bin/` directory to the `libexec/` directory in CMake installs and
binary releases. The goal is to simplify the contents of `bin/` for end users
while still making all binaries available when needed. After this change, the
binaries remaining in `bin/` are:
- bitcoin
- bitcoin-cli
- bitcoind
- bitcoin-qt
- bitcoin-tx
- bitcoin-util
- bitcoin-wallet
And the binaries that are moved to `libexec/` are:
- bench_bitcoin
- bitcoin-chainstate(*)
- bitcoin-gui(***)
- bitcoin-node(***)
- test_bitcoin(**)
- test_bitcoin-qt
(*) bitcoin-chainstate was previously missing an install rule and was actually
not installed even when it was enabled.
(**) test_bitcoin is the only libexec/ binary that is currently included in
bitcoin binary releases. The others are only installed when building from
source with relevant cmake options enabled.
(***) bitcoin-node and bitcoin-gui are not currently built by default or
included in binary releases but both of these changes are planned and
implemented in #31802
a5ac43d98d1ad3ebed934f2c50208a85aae17e5e doc: Add release notes describing bitcoin wrapper executable (Ryan Ofsky)
258bda80c009a25d1f1bdeffccf9ed1ffde29cb2 doc: Mention bitcoin wrapper executable in documentation (Ryan Ofsky)
d2739d75c911c8bf73a4d3005c57add1ae4a67ae build: add bitcoin.exe to windows installer (Sjors Provoost)
ba649c00063a43b59a63db17b509179a658a8d9a ci: Run multiprocess tests through wrapper executable (Ryan Ofsky)
29bdd743bb843f8b8ed2e426b6df36e9d7e54215 test: Support BITCOIN_CMD environment variable (Ryan Ofsky)
9c8c68891b43053acfe7b8eb9d2e0d2bcfcb4e1e multiprocess: Add bitcoin wrapper executable (Ryan Ofsky)
5076d20fdb70a4bfafc4bdfe8293e347cb6bfa78 util: Add cross-platform ExecVp and GetExePath functions (Ryan Ofsky)
Pull request description:
Intended to make bitcoin command line features more discoverable and allow installing new multiprocess binaries in libexec/ instead of bin/ so they don't cause confusion.
Idea and implementation of this were discussed in https://github.com/bitcoin/bitcoin/issues/30983.
---
Initial implementation of this feature is deliberately minimal so the UX can evolve in response to feedback and there are not too many details to debate and discuss in a single PR. But many improvements are possible or planned:
- Adding manpage and bash completions.
- Showing nicer error messages that detect if an executable isn't installed and suggest how to fix [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2073194474)
- Showing wrapper command lines in subcommand in help output [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2077800405). This could be done conditionally as suggested in the comment or be unconditional.
- Showing wrapper command lines in subcommand error output. There is a bitcoin-cli error pointed out in [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091152243) that is needlessly confusing.
- Integrating help so `bitcoin help subcommand` invokes `bitcoin subcommand -h`. `bitcoin -h subcommand` should also be supported and be equivalent [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2093116725)
- Adding support for `bitcoin-util` subcommands. Ideal interface would probably be more like `bitcoin grind` not `bitcoin util grind` but this has been punted for now. Supporting subcommands directly would require some ArgsManager modifications
- Adding a dedicated python functional test for the wrapper. Right now there is some CI coverage by setting the `BITCOIN_CMD` variable, but this doesn't cover things like the help output and version output, and support for different directory layouts.
- Better `--multiprocess` (`-m`) / `--monolithic` (`-M`) default selection. Right now, default is monolithic but it probably makes sense to chose more intelligently depending on whether -ipc options are enabled and what binaries are available.
- Maybe parsing `bitcoin.conf` and supporting options to control wrapper behavior like custom locations or preferences or aliases.
- Better command command line usability. Allow combining short options like (`-ah`). Allow fuzzy matching of subcommands or suggestions if you misspell. (suggested by stickies in review club)
- Not directly related to this PR but `bitcoin-cli named` implementation used by the wrapper should do a better job disambiguating named arguments from base64 arguments ending in = as pointed out in [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091886628)
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722). A review club meeting for it took place in https://bitcoincore.reviews/31375
ACKs for top commit:
Sjors:
utACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
achow101:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
vasild:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
theStack:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
ismaelsadeeq:
fwiw my last review implied an ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
hodlinator:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
Tree-SHA512: 570e6a4ff8bd79ef6554da3d01f36c0a7c6d2dd7dace8f8732eca98f4a8bc2284474a9beadeba783114fe2f3dd08b2041b3da7753bae0b7f881ec50668cb821f
