348e79f7c6 lint: Include test_utxo_snapshots in lint_shell (Fabian Jahr)
Pull request description:
jamesob excluded `test_utxo_snapshots.sh` from the shell linter with this explanation: "Add the script to the shellcheck exception list since the quoted variables rule needs to be violated in order to get bitcoind to pick up on $EARLY_IBD_FLAGS." However, macrofake pointed out that single lines can be excluded from linting.
This fixes one fixable rule violation, excludes the rest of the offending lines from the linter and then removes the exclusion of the `test_utxo_snapshots.sh` file. Also adds documentation.
ACKs for top commit:
Empact:
ACK 348e79f7c6
maflcko:
lgtm ACK 348e79f7c6
pablomartin4btc:
tACK 348e79f7c6
Tree-SHA512: a904cc1cc3c94488dfbd39ea69a3ef17259f991708a797009001669448fef81eed086ecbce1ec433988d88baef293849698e2e0eb86a969b949cc7ef93af7b4b
092daa2f95 contrib: add test for macOS linker version to symbol-check (fanquake)
cefbf0bc20 depends: update LD64_VERSION to 711 (fanquake)
Pull request description:
I forgot to do this in 7d5815293e.
Add a test so it's impossible to forget.
ACKs for top commit:
TheCharlatan:
utACK 092daa2f95
achow101:
ACK 092daa2f95
jarolrod:
ACK 092daa2f95
hebasto:
ACK 092daa2f95.
laanwj:
ACK 092daa2f95
Tree-SHA512: 37f0bdfd6607a7760eabe5efe279532ba0c59c0915161e08d5e3b9a0b7705839d62537d6e17406062f6a0a1db5407575da7cd671e9cb916e422e77f5649c6e2b
36a3004a41 devtools: test_utxo_snapshots.sh sleep cleanup and documentation (Fabian Jahr)
Pull request description:
There were bare sleep statements in the script where it was unclear why they were needed and I think they could also be replaced by existing helpers.
ACKs for top commit:
Sjors:
utACK 36a3004a41
pablomartin4btc:
utACK 36a3004a41
Tree-SHA512: b6e2fc69cd7babcfa7f413f11304f4d648d6d64b3b526862664ccedb0016ad182b9e886aa4e8c33315e4c18824512e11a4fd6365f9c3c95093967d3ef7687e62
3d420d8f28 Add instructions for headerssync-params.py to release-process.md (Pieter Wuille)
53d7d35b58 Update parameters in headerssync.cpp (Pieter Wuille)
7899402cff Add headerssync-params.py script to the repository (Pieter Wuille)
Pull request description:
Builds upon #25946, as it incorporates changes based on the selected values there.
This adds the headerssync tuning parameters optimization script from https://gist.github.com/sipa/016ae445c132cdf65a2791534dfb7ae1 to the repository, updates the parameters based on its output, and adds release process instructions for doing this update in the future.
A few considerations:
* It would be a bit cleaner to have these parameters be part of `CChainParams`, but due to the nature of the approach, it really only applies to chains with unforgeable proof-of-work, which we really can only reasonably expect from mainnet, so I think it's fine to keep them local to `headerssync.cpp`. Keeping them as compile-time evaluatable constants also has a (likely negligible) performance impact (avoiding runtime modulo operations).
* If we want to make sure the chainparams and headerssync params don't go out of date, it could be possible to run the script in CI, and and possibly even have the parameters be generated automatically at build time. I think that's overkill for how unfrequently these need to change, and running the script has non-trivial cost (~minutes in the normal python interpreter).
* A viable alternative is just leaving this out-of-repo entirely, and just do ad-hoc updating from time to time. Having it in the repo and release notes does make sure it's not forgotten, though adds a cost to contributors/maintainers who follow the process.
ACKs for top commit:
ajtowns:
reACK 3d420d8f28
Tree-SHA512: 03188301c20423c72c1cbd008ccce89b93e2898edcbeecc561b2928a0d64e9a829ab0744dc3b017c23de8b02f3c107ae31e694302d3931f4dc3540e184de1963
360b917674 contrib/bash-completions: use package naming conventions (Erik Arvstedt)
Pull request description:
#### Copy of commit msg
This naming scheme supports auto-detection and on-demand loading of completions.
See
ba109693ee/README.md (faq),
section "Where should I put it to be sure that interactive bash shells will find it and source it", keyword `foo.bash`.
Previously, distro package maintainers had to rename these files manually.
ACKs for top commit:
willcl-ark:
ACK 360b917674
Tree-SHA512: 6dd1f62309e877402fac2f7aec785d3053a1fd3fdeae38abc961c3f61269eb664a2489b6fa8294bd706d8ac387c7af3ac66cd7a852810f69371fa8ae991afacb
Add the script to the shellcheck exception list since the
quoted variables rule needs to be violated in order to get
bitcoind to pick up on $CHAIN_HACK_FLAGS.
This naming scheme supports auto-detection and on-demand loading of completions.
See
ba109693ee/README.md (faq),
section "Where should I put it to be sure that interactive bash shells will find it and source it".
Previously, distro package maintainers had to rename these files manually.
Using the new time-machine results in warnings about consistently using
keyword arguments:
```bash
guix environment: warning: 'cross-kernel-headers' must be used with keyword arguments
guix environment: warning: 'cross-libc' must be used with keyword arguments
```
This is required for bumping the time-machine, for compatibility with
OpenSSL:
oscrypto: openssl backend, 1.2.1, /tmp/guix-build-python-oscrypto-1.2.1.drv-0/source/oscrypto
Traceback (most recent call last):
File "/tmp/guix-build-python-oscrypto-1.2.1.drv-0/source/oscrypto/_openssl/_libcrypto_ctypes.py", line 304, in <module>
libcrypto.EVP_PKEY_size.argtypes = [
File "/gnu/store/9dkl9fnidcdpw19ncw5pk0p7dljx7ijb-python-3.10.7/lib/python3.10/ctypes/__init__.py", line 387, in __getattr__
func = self.__getitem__(name)
File "/gnu/store/9dkl9fnidcdpw19ncw5pk0p7dljx7ijb-python-3.10.7/lib/python3.10/ctypes/__init__.py", line 392, in __getitem__
func = self._FuncPtr((name_or_ordinal, self))
AttributeError: /gnu/store/2hr7w64zhr6jjznidyc2xi40d5ynhj9c-openssl-3.0.8/lib/libcrypto.so.3: undefined symbol: EVP_PKEY_size. Did you mean: 'EVP_PKEY_free'?
Refactor our glibc 2.27 to be a single 'package', and avoid the use of
`package-with-extra-configure-variable`. This also lets us drop the
`enable_werror` workaround, and just use --disable-werror directly.
Employ the same workaround as the Guix glibc, to avoid a "permission
denied" failure during build:
```bash
make subdir=sunrpc -C sunrpc ..=../ subdir_install
make[2]: Entering directory '/tmp/guix-build-glibc-cross-x86_64-linux-gnu-2.27.drv-0/source/sunrpc'
.././scripts/mkinstalldirs /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/rpc
mkdir -p -- /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/rpc
/gnu/store/kvpvk5wh70wdbjnr83hh85rg22ysxm9h-coreutils-8.32/bin/install -c -m 644 rpc/netdb.h /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/rpc/netdb.h
.././scripts/mkinstalldirs /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/nfs
mkdir -p -- /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/nfs
/gnu/store/kvpvk5wh70wdbjnr83hh85rg22ysxm9h-coreutils-8.32/bin/install -c -m 644 ../sysdeps/unix/sysv/linux/nfs/nfs.h /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/nfs/nfs.h
/gnu/store/kvpvk5wh70wdbjnr83hh85rg22ysxm9h-coreutils-8.32/bin/install -c -m 644 /tmp/guix-build-glibc-cross-x86_64-linux-gnu-2.27.drv-0/build/gnu/lib-names-64.h /gnu/store/ga8jciqrd5lh52m572x3mk4q1smf5agq-glibc-cross-x86_64-linux-gnu-2.27/include/gnu/lib-names-64.h
/gnu/store/kvpvk5wh70wdbjnr83hh85rg22ysxm9h-coreutils-8.32/bin/install -c -m 644 etc.rpc /etc/rpc
/gnu/store/kvpvk5wh70wdbjnr83hh85rg22ysxm9h-coreutils-8.32/bin/install: cannot create regular file '/etc/rpc': Permission denied
make[2]: *** [Makefile:197: /etc/rpc] Error 1
```
Split out of #27897. This is some refactoring to the Windows Guix build
that facilitates bumping our Guix time-machine. Namely, avoiding
`package-with-extra-configure-variable`, which is non-functional in the
newer time-machine, see https://issues.guix.gnu.org/64436.
At the same time, consolidate our Windows GCC build into mingw-w64-base-gcc.
Rename `gcc-10-remap-guix-store.patch` to avoid changing it whenever GCC changes.
We move the old `building-on` inside `explicit-cross-configure`, so that
non-windows builds continue to work. Note that `explicit-cross-configure`
will be going away entirely (see #27897).
fa60fa3b0c bitcoin-tidy: Apply bitcoin-unterminated-logprintf to spkm as well (MarcoFalke)
faa11434fe refactor: Enable all clang-tidy plugin bitcoin tests (MarcoFalke)
fa6dc57760 refactor: Enforce C-str fmt strings in WalletLogPrintf() (MarcoFalke)
fa244f3321 doc: Fix bitcoin-unterminated-logprintf tidy comments (MarcoFalke)
Pull request description:
All fmt functions only accept a raw C-string as argument.
There should never be a need to pass a format string that is not a compile-time string literal, so disallow it in `WalletLogPrintf()` to avoid accidentally introducing it.
Apart from consistency, this also fixes the clang-tidy plugin bug https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1286821141.
ACKs for top commit:
theuni:
ACK fa60fa3b0c
Tree-SHA512: fa6f4984c50f9b34e850bdfee7236706af586e512d866cc869cf0cdfaf9aa707029c210ca72d91f85e75fcbd8efe0d77084701de8c3d2004abfd7e46b6fa9072
bb3263d3e3 bitcoin-tidy: fix macOS build (Cory Fields)
Pull request description:
[LLVM uses these options](https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/HandleLLVMOptions.cmake#L178) for building as well, so there's precedent.
Also fix the shared library extension which was incorrectly being set to dylib.
Thanks to jonatack for reporting and debugging.
ACKs for top commit:
jonatack:
ACK bb3263d3e3 tested with arm64 macos 13.5, llvm 16.0.6 and cmake 3.27.2
Tree-SHA512: de7bfd497f38f1565a14d217d0b057cbfa788bdda702b5942b7f0b55947ae5e1c05af13e7d6a073ed036bc4db57035868f180034508b6e084ab9b901a5baaf2f
An LLVM installation will have `llvm-config` available to query for
info. Ask it for the `--cmakedir`, and use that in our bitcoin-tidy
example, rather than listing multiple different (potential) paths per
distro/OS etc.
7f96638723 contrib: add macOS fixup_chains check to security-check (fanquake)
3dca683cb7 build: support -no_fixup_chains in ld64 (fanquake)
Pull request description:
Followup to #27676, adding the check for chained fixups.
Somewhat annoyingly, we have to patch support for `-no_fixup_chains` into ld64. As it doesn't seem to have been added [until a later version](59a99ab603/src/ld/Options.cpp (L4172)).
Guix Build:
```bash
0e17d462808f86aa7157e27a957da88fd1adeb491ad6c01138aca93e5ad1d018 guix-build-7f96638723a0/output/arm64-apple-darwin/SHA256SUMS.part
ceb208e6374f5d7367b73128e90ca6eaeea15d50c69e49c8cf75b47212525ad7 guix-build-7f96638723a0/output/arm64-apple-darwin/bitcoin-7f96638723a0-arm64-apple-darwin-unsigned.dmg
e31663554cfde8a37a9f3438c9c895dde94b90ff87e28f12f78be71ef6421d93 guix-build-7f96638723a0/output/arm64-apple-darwin/bitcoin-7f96638723a0-arm64-apple-darwin-unsigned.tar.gz
68a7bbc42418641eab391a85725b5c2f3c46d38a7acc07e7a8cef98909be07ec guix-build-7f96638723a0/output/arm64-apple-darwin/bitcoin-7f96638723a0-arm64-apple-darwin.tar.gz
38d966ad93e7384f4f1ce16faded003a675ecce7be1987e6c4eee8e4b82c0432 guix-build-7f96638723a0/output/dist-archive/bitcoin-7f96638723a0.tar.gz
9d314f595d897a715a321a9fba0d552220fbd4bf69aff84eb8c0001cdb48234f guix-build-7f96638723a0/output/x86_64-apple-darwin/SHA256SUMS.part
c218ebfd0e96348c4912e6d522492b621bb043ef45b75105ff1fde979d1004d0 guix-build-7f96638723a0/output/x86_64-apple-darwin/bitcoin-7f96638723a0-x86_64-apple-darwin-unsigned.dmg
1c5ff7fa82f5c76d7d8b9582ad5202f4a82a917102ecafdc3c1fb7b783f6bc3e guix-build-7f96638723a0/output/x86_64-apple-darwin/bitcoin-7f96638723a0-x86_64-apple-darwin-unsigned.tar.gz
15fb01e5afcc842db6a3e793b42c70c05ce07bec79e0d2d605e241901ff9f639 guix-build-7f96638723a0/output/x86_64-apple-darwin/bitcoin-7f96638723a0-x86_64-apple-darwin.tar.gz
```
ACKs for top commit:
theuni:
utACK 7f96638723.
hebasto:
ACK 7f96638723, I have reviewed the code and the patch, and they look OK.
TheCharlatan:
ACK 7f96638723
Tree-SHA512: 7f94710460f54b2afe3c9f5d57107b71436c59b799b15f78e5e3011c3c4f6b23a3acc1008eccea9c22226a200774c82900bad6c6236ab6c5c48a17dec3f2d5a2
6c97757a48 script: appease spelling linter (Jon Atack)
1316119ce7 script: update ignored-words.txt (Jon Atack)
146c861da2 script: update linter dependencies (Jon Atack)
92408224a4 test: fix PEP484 no implicit optional argument types errors (Jon Atack)
f86a301433 script, test: add missing python type annotations (Jon Atack)
Pull request description:
With these updates, `./test/lint/lint-python.py` and `./test/lint/lint-spelling.py` should be green again for developers using relatively recent Python dependencies, in particular mypy 0.991 (released 11/2022) and later. Please see the commit messages for details.
ACKs for top commit:
fanquake:
ACK 6c97757a48
Tree-SHA512: 8a46a4d36d5978affdcecf4f2ace20ca1b52d483e098304911a2169afe60ccb9b042fa90c04b762d94f3ce53d2cafe6f24476ae839867a770c7f31e7e7242d99
Fix warnings for these files when ./test/lint/lint-python.py is run using
mypy 0.991 (released 11/2022) and later:
"By default the bodies of untyped functions are not checked, consider using
--check-untyped-defs [annotation-unchecked]"
For details, see:
https://mypy-lang.blogspot.com/2022/11/mypy-0990-released.html
This change improves the maintainability of the manifest:
(1) It allows to remove the module when the specified symbols are no
longer used.
(2) It prevents accidental use of other symbols, such as `bash`
instead of `bash-minimal`.
