zmq seems to be an alias for pyzmq, and the project page,
https://pypi.org/project/zmq/, states "You are probably looking for
pyzmq.". So switch to pyzmq, which is what we document, and use in all
other jobs.
fad585b6e5 test: Wait for node exit after crash in verify_utxo_hash (MarcoFalke)
faf1475514 ci: Exclude feature_dbcrash.py under --v2transport --usecli (MarcoFalke)
fac27d702f test: Fix feature_dbcrash.py --usecli intermittent error (MarcoFalke)
fa09de8b68 test: [refactor] Simplify submit_block_catch_error (MarcoFalke)
Pull request description:
This fixes a small intermittent issue that snuck in via commit fa8d4d5c35.
Generally, it seems tedious and brittle trying to enumerate all possible exception types on all platforms and all test configs, all possible errno values, etc.
So just treat any `Exception` as crash, and confirm it in the test. The test would still fail, if a crash did not happen after an Exception, but the failure may be minimally more tedious to debug. I think this is fine, because failures should be rare and having simpler and more flexible test code is preferable.
----
Also, disable the test for now in CI, because it is quite slow.
ACKs for top commit:
willcl-ark:
ACK fad585b6e5
Tree-SHA512: 6ff69a53908b22904780f29def473f8e26c5b608d89420ac76768d06ea3e3394b5d3f4d488100f9d47f943ff5778a555302ccdaebc11fda7c009205b6a6ca05c
The test should now be passing, but it is slow.
For example, the commit that enabled the test, took ~47 minutes on a
fast CPU, but using a heavy debug build:
https://github.com/bitcoin/bitcoin/actions/runs/26434786214/job/77815064664?pr=35363#step:11:4101
...
Model name: AMD Ryzen 9 7950X3D 16-Core Processor
...
C++ compiler .......................... GNU 15.2.0, /usr/bin/g++
CMAKE_BUILD_TYPE ...................... Debug
Preprocessor defined macros ........... DEBUG DEBUG_LOCKORDER DEBUG_LOCKCONTENTION RPC_DOC_CHECK ABORT_ON_FAILED_ASSUME _GLIBCXX_DEBUG _GLIBCXX_DEBUG_PEDANTIC _LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG
C++ compiler flags .................... -m64 -O0 -ftrapv -O1 -g3 -g3 -std=c++20 ...
...
feature_dbcrash.py | ✓ Passed | 2806 s
1e5d3b4f0d doc: add release note for mining option validation (Sjors Provoost)
0317f52022 ci: enforce iwyu for touched files (Sjors Provoost)
8c58f63578 refactor: have mining files include what they use (Sjors Provoost)
3bb6498fb0 mining: store block create options in NodeContext (Sjors Provoost)
4637cd157d mining: reject invalid block create options (Sjors Provoost)
8daac1d6eb mining: add block create option helpers (Sjors Provoost)
128da7c3ff miner: add block_max_weight to BlockCreateOptions (Sjors Provoost)
fa81e51eae mining: parse block creation args in mining_args (Sjors Provoost)
020166080c mining: use interface for tests, bench and fuzzers (Sjors Provoost)
44082bea47 interfaces: make Mining use const NodeContext (Sjors Provoost)
d4368e059c move-only: add node/mining_types.h (Sjors Provoost)
6aeb1fbea2 test: cover IPC blockmaxweight policy (Sjors Provoost)
63b23ea1e9 test: regression test for waitNext mining policy (Sjors Provoost)
24750f8b31 test: add createNewBlock failure helper (Sjors Provoost)
63ee9cd15b test: misc interface_ipc_mining.py improvements (Sjors Provoost)
Pull request description:
Although this PR is primarily a refactor, _there are behavior changes_ documented in the release note:
- the IPC mining interface now rejects out-of-range block template options instead of silently clamping them;
- startup now rejects `-blockmaxweight` values lower than `-blockreservedweight`, instead of allowing them to be clamped later.
The interaction between node startup options like `-blockreservedweight` and runtime options, especially those passed via IPC, is confusing.
They're combined in `BlockAssembler::Options`, which this PR gets rid of in favour of `BlockCreateOptions`.
`BlockCreateOptions` is used by interface clients. As before, IPC clients have access to a safe / sane subset, whereas RPC and test code can use all fields. The same type is also used to store mining defaults parsed once during node startup in `NodeContext`.
The maximum block weight setting (`block_max_weight`) is optional. When read from startup options it matches `-blockmaxweight`; when provided by callers it is a runtime override. `Merge()` fills unset fields from startup defaults while preserving caller-provided values.
This all happens in commits `mining: add block create option helpers` and `mining: store block create options in NodeContext`, and requires some preparation to keep things easy to review.
We get rid of `BlockAssembler::Options` but this is used in many tests. Since large churn is inevitable, we might as well switch all tests, bench and fuzzers over to the Mining interface. The `mining: use interface for tests, bench and fuzzers` commit does that, dramatically reducing direct use of `BlockAssembler`. Two exceptions are documented in the commit message. Because `test_block_validity` wasn't available via the interface and the block_assemble benchmark needs it, it's moved from `BlockAssembler::Options` to `BlockCreateOptions` (still not exposed via IPC).
We need access to mining related structs from both the miner and node initialization code. To avoid having to pull in all of `BlockAssembler` for the latter, the `move-only: add node/mining_types.h` commit introduces `node/mining_types.h` and moves `BlockCreateOptions`, `BlockWaitOptions` and `BlockCheckOptions` there from `src/node/types.h`.
I considered also moving `DEFAULT_BLOCK_MAX_WEIGHT`, `DEFAULT_BLOCK_RESERVED_WEIGHT`, `MINIMUM_BLOCK_RESERVED_WEIGHT` and `DEFAULT_BLOCK_MIN_TX_FEE` there from `policy.h`, since they are distinct from relay policy and not needed by the kernel. But this seems more appropriate for a follow-up and requires additional discussion.
---
I kept variable renaming and other formatting changes to a minimum to ease review with `--color-moved=dimmed-zebra`.
## Commit summary
Tests and test cleanup:
- `test: misc interface_ipc_mining.py improvements`
- `test: add assert_create_fails helper`
- `test: regression test for waitNext mining policy`
- `test: cover IPC blockmaxweight policy`
Refactoring test/bench/fuzz callers:
- `interfaces: make Mining use const NodeContext`
- `mining: use interface for tests, bench and fuzzers`
Moving mining interface types:
- `move-only: add node/mining_types.h`
Separating startup defaults from runtime options:
- `mining: parse block creation args in mining_args`: adds `node/mining_args.{h,cpp}` and moves mining option parsing out of `init.cpp`, without storing the parsed values yet.
- `miner: add block_max_weight to BlockCreateOptions`: moves the runtime maximum block weight setting into `BlockCreateOptions` as an optional value, so it can later be defaulted from startup args when unset.
- `mining: add block create option helpers`: centralizes block template option defaulting and merging, removes `BlockAssembler::Options`, and preserves behavior except for dropping the `Specified ` prefix from startup option error messages.
- `mining: reject invalid block create options`: checks typed `BlockCreateOptions` before block template creation, so invalid runtime options are rejected instead of silently clamped. Startup validation also rejects `-blockmaxweight` values lower than `-blockreservedweight`.
- `mining: store block create options in NodeContext`: stores the startup mining options in `NodeContext` as `BlockCreateOptions`, so startup defaults and runtime overrides can be merged with the same option type.
Include hygiene, CI and release note:
- `refactor: have mining files include what they use`
- `ci: enforce iwyu for touched files`
- `doc: add release note for mining option validation`
ACKs for top commit:
w0xlt:
reACK 1e5d3b4f0d
sedited:
ACK 1e5d3b4f0d
ryanofsky:
Code review ACK 1e5d3b4f0d. Looks good, thanks for the updates!
Tree-SHA512: 28c715023cb78f02775caa787b243c994bd0f8ce4559afc8db9301e93400ebbc74963626a4afe65ae15bcc16b9192d051a745839f4c804848d50746ea5a224b4
7777a92a92 ci: Use path with spaces on windows as well (MarcoFalke)
fac6c4270d ci: Put space and non-ASCII char in scratch dir (MarcoFalke)
fa38759823 ci: Require $FILE_ENV (MarcoFalke)
Pull request description:
It seems unlikely that many users have a space in their paths, but it seems a use-case worth enough to be tested by CI, so that it does not have to be done manually. Ref https://github.com/bitcoin/bitcoin/pull/33929#discussion_r2590523065 / https://mirror.b10c.me/bitcoin-bitcoin/33929/#discussion_r2590523065
So do that here, and also add a non-ASCII char while touching.
Also, fix all tests that are broken and assume no space exists in paths.
ACKs for top commit:
hebasto:
ACK 7777a92a92.
sedited:
ACK 7777a92a92
Tree-SHA512: eceb1f6c932c6966cdca8ca8df750081ec5134db5e5f558f7d955716409117bec7c8585d75865e2c98bc1ae7394f3ce64dff87bcebe1e68591afaeef1831d6dd
75cf9708a0 ci: add one more routable address to the VMs (docker containers) (Vasil Dimov)
1b93983bf5 test: make feature_bind_port_(discover|externalip).py auto-detect the skip condition (Vasil Dimov)
Pull request description:
`feature_bind_port_discover.py` and `feature_bind_port_externalip.py` require a routable address on the machine to run. Since that was not predictably available on CI, those tests required a manual setting up of IP addresses (e.g. using `ifconfig`) and then running the tests with a command line option telling them that the addresses are set up. The tests were not run in CI and [got rot](https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2497792487).
Change that to auto-detect, from the tests, whether the needed IP addresses are present and if yes, run the test, otherwise skip it. Also change the CI to configure the needed addresses when running the functional tests. This way the tests will be run regularly on CI.
Fixes: https://github.com/bitcoin/bitcoin/issues/31336
ACKs for top commit:
willcl-ark:
ACK 75cf9708a0
frankomosh:
Tested ACK 75cf9708a0. Built from source.
ryanofsky:
Code review ACK 75cf9708a0. Tested locally with and without the special addresses, and the detection seems to work well.
Tree-SHA512: 252911a37a06764f644a1a83c808f5255ac3bc74919426afa5d082c59e1ea924196354735f229d381cb5aff2340e001c2240bbadc8b5f27e5321fb4cfaef0fdb
Also, add a missing quote around -DCMAKE_INSTALL_PREFIX to avoid word
splitting.
Otherwise, cmake would warn:
```
+ cmake -S /home/runner/work/_temp -B /home/runner/work/_temp/build -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DCMAKE_INSTALL_PREFIX=/home/runner/work/_temp/ci/scratch_ ₿🧪_/out -Werror=dev --preset=dev-mode -DSANITIZERS=address,float-divide-by-zero,integer,undefined -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_FLAGS=-ftrivial-auto-var-init=pattern -DCMAKE_CXX_FLAGS=-ftrivial-auto-var-init=pattern -DCMAKE_EXE_LINKER_FLAGS=-fuse-ld=mold -DAPPEND_CXXFLAGS=-std=c++23 '-DAPPEND_CPPFLAGS=-DARENA_DEBUG -DDEBUG_LOCKORDER'
CMake Warning:
Ignoring extra path from command line:
"₿🧪_/out"
```
Also, allow spaces in the debug log file regex in a test.
Otherwise, the test would fail:
```
TestFramework (ERROR): Unexpected exception:
Traceback (most recent call last):
File "./test/functional/test_framework/test_framework.py", line 142, in main
self.run_test()
~~~~~~~~~~~~~^^
File "./test/functional/feature_logging.py", line 40, in run_test
self.nodes[0].assert_start_raises_init_error([f"-debuglogfile={invalidname}"], exp_stderr, match=ErrorMatch.FULL_REGEX)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "./test/functional/test_framework/test_node.py", line 743, in assert_start_raises_init_error
self._raise_assertion_error(
~~~~~~~~~~~~~~~~~~~~~~~~~~~^
'Expected message "{}" does not fully match stderr:\n"{}"'.format(expected_msg, stderr))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "./test/functional/test_framework/test_node.py", line 229, in _raise_assertion_error
raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Expected message "Error: Could not open debug log file \S+$" does not fully match stderr:
"Error: Could not open debug log file /Users/runner/work/bitcoin-core-with-ci/bitcoin-core-with-ci/repo_archive/ci/scratch_ ₿🧪_/test_runner/test_runner_₿_🏃_20260218_095938/feature_logging_31/node0/regtest/foo/foo.log"
```
Also, add missing quotes in a test. Otherwise, the test would fail:
```
455/455 - feature_notifications.py failed, Duration: 402 s
stdout:
TestFramework (INFO): Initializing test directory /home/runner/work/_temp/ci/scratch_ ₿🧪_/test_runner/test_runner_₿_🏃_20260218_113529/feature_notifications_128
TestFramework (INFO): test -blocknotify
TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
self.wait_until(lambda: len(os.listdir(self.blocknotify_dir)) == block_count, timeout=10)
'''
d084bc88be doc: clarify IWYU workflow (Lőrinc)
7c7cec4567 ci: update IWYU patch reference (Lőrinc)
Pull request description:
### Problem
This was prompted by https://github.com/bitcoin/bitcoin/pull/34435#discussion_r3123255248, where it was not clear to me how (and where) exceptional IWYU cases should be documented.
### Fix
This PR documents the IWYU CI wrapper as the reproducible local entrypoint.
The developer notes now recommend reducing suspected IWYU false positives to a minimal upstream reproducer, treat `IWYU pragma` as a narrow workarounds, and ask for nearby rationale comments on non-obvious IWYU pragma use. An example comment was also added.
The IWYU patch comment is also updated to point at the current `clang_22` include picker reference.
### Reproducer
Create a dummy commit on top that adds an unused include, then run the command from the developer notes.
Without the dummy commit, the command should pass.
<details><summary>IWYU demo commit</summary>
```diff
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
--- a/src/kernel/bitcoinkernel.cpp(revision c92b329e7b7d49476b5977d26c24d7c4982c6024)
+++ b/src/kernel/bitcoinkernel.cpp(revision ad2c5ba2ba69156e77061c1e6c098b725c28f322)
@@ -43,6 +43,7 @@
#include <functional>
#include <list>
#include <memory>
+#include <vector>
#include <span>
#include <stdexcept>
#include <string>
```
</details>
> [!NOTE]
> After repeated failing runs, `docker container rm -f ci_native_iwyu` may be needed because the local CI wrapper can leave the detached container running when the inner test command fails.
ACKs for top commit:
hebasto:
ACK d084bc88be.
sedited:
ACK d084bc88be
Tree-SHA512: 0aac42d468a1fdfa9f4a3856372e05fb43ec9f0973aeb3a4194fff948fc61e8e72e3b280cde10e74b8da88b6cff93962b3a7f7390eb042113ef92aa6b51d6d8f
a49bc1e24e ci: add --extended when using --usecli (Pol Espinasa)
5603ae0ffa test: fix send_batch_request to pass callables when using --usecli (Pol Espinasa)
Pull request description:
**First commit**
This commit fixes an error that made feature_index_prune.py fail if --usecli was used.
The test was failing because `node.batch(data)` was called with `data` being a dict. This worked in the normal scenario because `AuthServiceProxy.batch()` expects a list of petitions in the form of a dict. But `TestNodeCLI.batch()` expects callables and not a dict. When `--usecli` is used the test fails with an error `TypeError: 'dict' object is not callable`.
This is fixed by using `get_request()` which returns a lambda function if `--usecli` is used and returns a dict if not.
The `assert` is also changed because before this PR, the requests, constructed by hand were not specifiying the json-rpc version. By default if no version is specified we use version 1.0 which always returns `error: none` if there is no error. However, in version 2.0, it does not include an error key if there is no error. By using `get_request()` the requests are done in version 2.0 so there's no key error.
**Second commit**
The current CI doesn't cover the case of running all tests with --extended if using --usecli.
This led to a test failing (feature_index_prune.py) if run with --usecli and the CI not catching it.
See https://github.com/bitcoin/bitcoin/pull/34991 for context.
This commit improves the CI test coverage by also running now all functional tests (--extended) with the flag --usecli.
<details>
<summary>First "wrong" approach</summary>
Fixes two bugs that make `feature_index_prune.py` fail if `--usecli` is used.
1. Makes `TestNodeCLI.batch()` response equivalent to what a JSON-RPC response would look like by adding `error=None` in the response. The lack of `error` in the response was giving a `KeyError` message.
2. Makes `send_batch_request()` compatible with --usecli. Before the PR it was passing dicts to `node.batch()`, but `TestNodeCLI.batch()` expects callables, not dicts.
</details>
ACKs for top commit:
Bicaru20:
Re-ACK a49bc1e24e.
sedited:
Re-ACK a49bc1e24e
Tree-SHA512: 75fca26cf120638ced1fe38e86d8e3efa7addb6d97fc801e34783efd2cf6417f4ded2ec6247b6dcbcdb3cf4f48c4858f0932cbaa3e836a973d53581e75470a3f
Also explicitly specify which addresses from the docker network to
assign to the VM.
With `1.1.1.5` and `1111:1111::5` set on the machine, the tests
`feature_bind_port_discover.py` and
`feature_bind_port_externalip.py` will run.
65379bb8d0 ci: add FreeBSD cross CI job (fanquake)
f44191f163 depends: build qrencode for Freebsd (fanquake)
7f7018738e depends: FreeBSD cross with Clang (fanquake)
6464f14081 depends: disable inotify in Freebsd Qt build (fanquake)
Pull request description:
Alternative to #33562, which was adding a native FreeBSD job; however that had issues with permissions/caching, as well as potential determinism issues. This adds a FreeBSD cross job using Linux and Clang.
Would close#33438. The same changes here could also be used to produce FreeBSD binaries out of Guix.
ACKs for top commit:
hebasto:
ACK 65379bb8d0. I've cross-compiled on Ubuntu 25.10 for FreeBSD 14.4 and 15.0. The former binaries (`bitcoind`, `test_bitcoin` and `bitcoin-qt`) were tested on FreeBSD 14.4 locally.
Tree-SHA512: 52a3edaa56fe40ca901416cb9e1af04a84505526edfa7309bfa40024baa7d3b1a05303659553d9fbcf1f49d4e3d42b415a1e2523d448b22724d1415a49331259
3129d4a693 ci: Rename `TIDY_LLVM_V` to `IWYU_LLVM_V` in IWYU-specific code (Hennadii Stepanov)
0b489886f8 ci: Upgrade IWYU to 0.26 compatible with Clang 22 (Hennadii Stepanov)
Pull request description:
This PR upgrades IWYU to [0.26](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.26) and removes mapping workarounds for issues that have been fixed upstream.
ACKs for top commit:
fanquake:
ACK 3129d4a693
Tree-SHA512: 9a926e489573d040423461c039ecda7636beb70e8214a02c1c594cbd5b89d26331455a9872c38993fa5ee6d27fdfefc2375dedc369721b2933411542a57a3884
fa70b9ebaa ci: Temporarily use clang in valgrind tasks (MarcoFalke)
faf3ef4ee7 ci: Clarify why valgrind task has gui disabled (MarcoFalke)
fadb77169b test: Scale feature_dbcrash.py timeout with factor (MarcoFalke)
Pull request description:
valgrind currently does not work on GCC compiled executables, due to an upstream bug. https://bugs.kde.org/show_bug.cgi?id=472329
So temporarily switch to clang, so that a long term solution can be figured out in the meantime.
ACKs for top commit:
l0rinc:
ACK fa70b9ebaa
fanquake:
ACK fa70b9ebaa - also checked that it doesn't currently work under aarch64.
Tree-SHA512: 2e7c7a709311efa7bf29c3f9b1db60886b189b2d2bfebb516062163d65f0d7e8de3b6fc21c94cd62f6bd7e786e9c36fba55c4bae956b849851eb8b08e772c03e
valgrind currently does not work on GCC -O2 compiled executables, which
contain std::optional use, due to an upstream bug. See
https://bugs.kde.org/show_bug.cgi?id=472329
One workaround could be to use -O1. However, that seems brittle, as
variantions of the bug were seen with -O1 as well.
So temporarily use clang in the valgrind CI tasks, because this also
allows to drop a false-positive suppression for:
-DCMAKE_CXX_FLAGS='-Wno-error=array-bounds'
Also, update the comment in contrib/valgrind.supp to mention the
background:
* GCC -O2 wasn't tested with the suppressions file, due to the mentioned
bug.
* Clang-17 (or later) on aarch64 wasn't tested due to bug
https://github.com/bitcoin/bitcoin/issues/29635 and the minimum
supported clang version is clang-17 right now.
* GUI isn't tested, because it requires a debug build, see the prior
commit.
This means the only tested config right now is the one mentioned in the
suppression file.
A build with system libs (or with a normal depends build) will fail
with:
```sh
$ valgrind --exit-on-first-error=yes --error-exitcode=1 --quiet ./bld-cmake/bin/test_bitcoin-qt
Detected locale "C" with character encoding "ANSI_X3.4-1968", which is not UTF-8.
Qt depends on a UTF-8 locale, and has switched to "C.UTF-8" instead.
If this causes problems, reconfigure your locale. See the locale(1) manual
for more information.
********* Start testing of AppTests *********
Config: Using QtTest library 6.10.2, Qt 6.10.2 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 15.2.0), ubuntu 26.04
PASS : AppTests::initTestCase()
QINFO : AppTests::appTests() Backing up GUI settings to "/tmp/test_common bitcoin/60d474ffae390f81657d/regtest/guisettings.ini.bak"
==18007== Conditional jump or move depends on uninitialised value(s)
==18007== at 0x12655E26: ???
==18007== by 0xCB28E7F: ???
==18007==
==18007==
==18007== Exit program on first error (--exit-on-first-error=yes)
```
A DEBUG=1 depends build would work, but that seems tedious for
questionable benefit.
d03e3be246 ci: check macos bundle structure and codesigning (fanquake)
66d80d57b4 macdeploy: use plugins dir to find plugins (fanquake)
ab137cbfe2 macdeploy: subprocess out to zip rather than shutil.make_archive (fanquake)
Pull request description:
Fix bundle format.
Add a CI check that codesigning works.
Fixes#34744.
ACKs for top commit:
Sjors:
tACK d03e3be246
hebasto:
ACK d03e3be246, tested on macOS Tahoe 26.3.1.
sedited:
ACK d03e3be246
Tree-SHA512: 5a7db896952edf338ff4fe8c934f1e1c992642850a99d5fafbb1212c6979601b3b72b6f3af880fb6f6ac8759cd4102e9f01792abb05410ceaf36cbffaec48e47
fafdb8f635 ci: Allow running iwyu ci in worktree (MarcoFalke)
fab73e213d ci: Reject unsafe execution of shell scripts (MarcoFalke)
Pull request description:
Currently, the iwyu CI fails to run in a git-worktree, or git-archive. This is due to the use of `git diff`.
Fix this by force-initializing a dummy git repo with a single dummy commit.
It may be possible to detect when `git diff` is not available in the directory, and only apply the fallback when needed, but the git history is not needed and it is easier to unconditionally apply the git init.
ACKs for top commit:
willcl-ark:
reACK fafdb8f635
hebasto:
ACK fafdb8f635, I have reviewed the code and it looks OK. Tested on Fedora 43.
sedited:
ACK fafdb8f635
Tree-SHA512: 572f1e2b9e215c2804095382498abb5b8636e3a49d5ba2a736b975e06afa2881d815b854a8a593d0f187c7c6b55034688e11f46d6814edfe7c29505197e80b18
24699fec84 doc: Add initial asmap data documentation (Fabian Jahr)
bab085d282 ci: Use without embedded asmap build option in one ci job (Fabian Jahr)
e53934422a doc: Expand documentation on asmap feature and tooling (Fabian Jahr)
6244212a55 init, net: Implement usage of binary-embedded asmap data (Fabian Jahr)
6202b50fb9 build: Generate ip_asn.dat.h during build process (Fabian Jahr)
634cd60dc8 build: Add embedded asmap data (Fabian Jahr)
Pull request description:
This is the final in a series of PRs that implement the necessary changes for embedding of asmap data into the binary. This last part add the initial asmap data, implements the build changes and adds further documentation.
Currently an asmap file needs to be acquired by there user from some location or the user needs to generate one themselves. Then they need to move the file to the right place in datadir or pass the path to the file as `-asmap=PATH` in order to use the asmap feature. The change here allows for builds to embed asmap data into the bitcoind binary which makes it possible to use the feature without handling of the asmap file by the user. If the user starts bitcoind with `-asmap` the embedded data will be used for bucketing of nodes.
The data lives in the repository at `src/node/data/ip_asn.dat` and can be replaced with a new version at any time. The idea is that the data should be updated with every release. By default the data at that location is embedded into the binary but there is also a build option to prevent this (`-DWITH_EMBEDDED_ASMAP=OFF`). In this case the original behavior of the `-asmap` option is maintained.
ACKs for top commit:
achow101:
ACK 24699fec84
sipa:
ACK 24699fec84
hodlinator:
ACK 24699fec84
Tree-SHA512: c2e33dbeea387efdfd3d415432bf8fa64de80f272c1207015ea53b85bb77f5c29f1dae5644513a23c844a98fb0a4bb257bf765f38b15bfc4c41984f0315b4c6a
The shell scripts are inherently unsafe, because they will install new
software packages, modify global configuration settings, write to the
root / or $HOME, and possibly modify the git repo.
The only safe way to run them is through the CI system itself, that is
the ci_exec python function.
The ci_exec funtion ensures that the user has set up a sandbox
externally and set DANGER_RUN_CI_ON_HOST=1 at their own risk, or that a
sandbox was set up with the given container_id, in which case it is safe
to set DANGER_RUN_CI_ON_HOST=1 for that sandbox.
Also, it is safe to set DANGER_RUN_CI_ON_HOST=1 when building the
sandbox image in ci/test_imagefile.
Then, the two shell scripts can reject early if unsafe execution is
detected.
d79249d279 ci: add chimera Linux LTO CI job (fanquake)
Pull request description:
Adds a CI config based on using [Chimera Linux](https://chimera-linux.org/). This might be interesting for any of the following:
* Chimera is based on LLVM & musl libc - we test both of these in isolation, but not together.
* No GNU components. I don't think we have an existing Linux CI job that doesn't have a gcc/stdlibc++ install. This exercises the depends logic for a fully LLVM/Clang/lld only build, including building the native tools (related to #33902).
* We don't currently have a job with LTO enabled (here using CMakes `CMAKE_INTERPROCEDURAL_OPTIMIZATION`, which is `-flto=thin` for LLVM/Clang). I think this is worth having generally (we do use LTO in some other places, like oss-fuzz). If runtime is too much of an issue, then it could also be dropped. (Chimera itself is also compiled with LTO).
QT in depends doesn't build (#32744), so is excluded for now.
Chimera has pointed out at least a few quirks, i.e #34390, #34408 and https://github.com/bitcoin/bitcoin/pull/29963#discussion_r2707922298.
ACKs for top commit:
maflcko:
lgtm ACK d79249d279
hebasto:
ACK d79249d279.
Tree-SHA512: 1174a7462bf2e7433a2c27a6cf398e94b05db42bb414629c71cf9f9a297ca269e173ae1b7517b30510b494b4397f918eef706d3c75c4286767c5557aeb6db4c7
b65a3d8009 iwyu: Fix patch to prefer `<cstdint>` (Hennadii Stepanov)
Pull request description:
The goal of the [patch](https://github.com/bitcoin/bitcoin/blob/master/ci/test/01_iwyu.patch) is to suggest C++ headers rather than their C counterparts. However, for fixed width integer types, the patched IWYU currently suggests `<cinttypes>` where `<cstdint>` is sufficient.
This PR fixes this behavior.
ACKs for top commit:
maflcko:
lgtm ACK b65a3d8009
furszy:
utACK b65a3d8009
willcl-ark:
utACK b65a3d8009
Tree-SHA512: 695efdd44b92a642401738572e49c8b6591aa4463d387107fdf3d2f7c9c4b39f4097cb82413752caf9e8890dcca7246a894e562a1dd17023b05a7e455705beac
-Werror is added to the previous releases job, given it runs on Ubuntu
22.04, which uses an older CMake.
`--compile-no-warning-as-error` can be used, if needed, in future, to
suppress the `CMAKE_COMPILE_WARNING_AS_ERROR` behaviour from a CI
config.
CMAKE_COMPILE_WARNING_AS_ERROR was added to CMake in 3.24.
See https://cmake.org/cmake/help/latest/prop_tgt/COMPILE_WARNING_AS_ERROR.html.
Co-authored-by: willcl-ark <will8clark@gmail.com>
The goal of the patch is to suggest C++ headers rather than their C
counterparts. However, for fixed width integer types, the patched IWYU
currently suggests `<cinttypes>` where `<cstdint>` is sufficient.
This change fixes this behavior.
