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)
'''
a39cc16b43 doc: Release note for addhdkey (Ava Chow)
89b9a01b4e wallet, rpc: Disallow importing unused() to wallets without privkeys (Ava Chow)
35bbee6374 wallet, rpc: Disallow import of unused() if key already exists (Ava Chow)
f3f8bcbd1d wallet: Add addhdkey RPC (Ava Chow)
82bc280de4 test: Simple test for importing unused(KEY) (Ava Chow)
80c29bc6f1 descriptor: Add unused(KEY) descriptor (Ava Chow)
Pull request description:
It is sometimes useful for the wallet to have keys that it can sign with but are not (initially) involved in any scripts, e.g. for setting up a multisig. Ryanofsky [suggested](https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1867721948) A `unused(KEY)` descriptor which allows for a key to be specified, but produces no scripts. These can be imported into the wallet, and subsequently retrieved with `gethdkeys`. Additionally, `listdescriptors` will output these descriptors so that they can be easily backed up.
In order to make it easier for people to add HD keys to their wallet, and to generate a new one if they want to rotate their descriptors, an `addhdkey` RPC is also added. Without arguments, it will generate a new HD key and add it to the wallet via a `unused(KEY)` descriptor. If provided a private key, it will construct the descriptor and add it to the wallet.
See also: https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866961865
Based on #29130 as `gethdkeys` is useful for testing this.
ACKs for top commit:
Sjors:
utACK a39cc16
rkrux:
lgtm ACK a39cc16b43
Tree-SHA512: c1288c792ab01ca2eaddd24b0e7d11c259cd59e79042465d0d1eb656fd559c1200dc19750b4d84acc762b5b599935a06df214c18226e662087842ea91ec3011b
6d86184a8b rpc: combinerawtransaction now rejects unmergeable transactions (Adam Andrews)
Pull request description:
Previously, combinerawtransaction would silently return the first tx when asked to combine unrelated txs. Now, it will check tx mergeability and throws a descriptive error if tx cannot be merged.
fixes #25980
ACKs for top commit:
nervana21:
tACK 6d86184a8b
achow101:
ACK 6d86184a8b
rkrux:
ACK [6d86184](6d86184a8b)
Tree-SHA512: 5caf983c5ab618a000f40b9ad698439d3e6217ec2dc593740443f47d90f8804a895f3054dd29bbcecdb48a61992b0d0afda7ec89591d768a44918648bbb6e20d
dc84a31014 wallet: remove fUpdate argument from AddToWalletIfInvolvingMe (rkrux)
94845df073 wallet: remove update_tx argument from SyncTransaction (rkrux)
6e796e1f47 wallet: remove fUpdate argument from ScanForWalletTransactions (rkrux)
54e4c0be8f wallet: remove update argument from RescanFromTime method (rkrux)
Pull request description:
This caught my attention while going through #32993.
The corresponding boolean arguments from CWallet methods related to
updating transactions during the blockchain scanning have been removed
because effectively these arguments were always passed as true making
the need for the boolean arguments unnecessary. The only falsy call sites
were in the unit tests that don't need to test scenarios that never happen
in actuality.
ACKs for top commit:
Bicaru20:
ACK dc84a31014. I also think that makes sense to remove the update argument if it is not used.
achow101:
ACK dc84a31014
sedited:
ACK dc84a31014
l0rinc:
untested code review ACK dc84a31014
Tree-SHA512: dfee793c88cc94d4a5b554e29710133ec4ad5e49455413245c7d7004ef71b618b0b2f821fa085415c34d80aeccd182c98e55223d84afbe8b408f64acac0571bb
0065f354a7 doc: clarify libfuzzer-nosan preset uses build_fuzz_nosan dir (ImMike)
Pull request description:
Adds a clarifying note next to the first mention of the `libfuzzer-nosan` preset in the Quickstart, pointing out that it uses a different build directory (`build_fuzz_nosan`, per [`CMakePresets.json` L54](https://github.com/bitcoin/bitcoin/blob/master/CMakePresets.json#L54)).
A reader following the quickstart with `--preset=libfuzzer-nosan` and then running `cmake --build build_fuzz` as shown would otherwise operate against the wrong (or empty) directory.
Pure docs; no code changes.
ACKs for top commit:
l0rinc:
ACK 0065f354a7
maflcko:
lgtm ACK 0065f354a7
sedited:
ACK 0065f354a7
Tree-SHA512: d73901112d259cec58746dff50fe3f9409e5b9826f0759f45478fe039bca851eb163036c60bdb215bfc66be79428b790742bbe8bc32b1ceaa2d6f80c17faf6d0
ca93ab808c doc: mention -DWITH_ZMQ=ON in BSD build guides (junbyjun1238)
Pull request description:
The BSD build guides currently state:
> If the package is installed, support will be compiled in.
Since `WITH_ZMQ` defaults to `OFF`, this is inaccurate: installing the dependency alone does not enable ZMQ support. Update the wording to mention the required `-DWITH_ZMQ=ON` CMake option, matching `doc/zmq.md`.
Docs-only change; no tests run.
ACKs for top commit:
maflcko:
lgtm ACK ca93ab808c
sedited:
ACK ca93ab808c
Tree-SHA512: d07b1b9748d8b6aa555c992608f8659b7e93d6587bbbb2170352342003e90a81592ddfa2abf3a26b8ebf1341142d8628cfc3faba0c4759655a91446afa5fc22a
Remove the option to signal or not signal for BIP 125 (Opt-in Full Replace-By-Fee).
By removing the option in the GUI it will fallback to the wallet configuration which by default is
true unless the user changed it using CLI.
The previous commit made CreateNewBlock() ignore the
include_dummy_extranonce flag (OP_0 is now appended only at heights
<= 16 to satisfy bad-cb-length, regardless of caller-supplied options).
Remove the now-unused field from BlockCreateOptions and clean up all
the call sites that still set it (RPC, bench, fuzz, test setups).
Drop the include_dummy_extranonce branch from the OP_0 padding
condition in CreateNewBlock(), so that the dummy extraNonce is
only appended when consensus actually requires it (heights <= 16,
where the BIP34 height push alone would yield a 1-byte scriptSig
and trigger bad-cb-length).
The include_dummy_extranonce option struct field is now unused by
the miner and is removed in the next commit. Callers still set it,
so that this commit compiles.
Regenerate the hardcoded coinbase / block hashes throughout the
unit and functional test suites and update the regtest assumeutxo
snapshot in chainparams.
Additional side-effects:
- Without the dummy extranonce, coinbase scriptSigs are 1 byte
shorter at heights > 16, making every block 1 byte smaller.
This shifts where block files wrap and therefore where pruning
boundaries land.
- feature_assumeutxo malleation cases:
- case 1: error message changes due to UTXO reordering, similar
to 8f2078af6a
- case 4: the corruption byte is swapped from \x82 to \x83
because \x82 happened to be the actual value at that
offset in the new snapshot.
Since #32420, createNewBlock has thrown `bad-cb-length` errors when called at
low block heights because `OP_0` padding stopped being added to coinbase
transactions. (#32420 did add an `include_dummy_extranonce` option which could
bypass this, but it was not exposed to IPC clients.) Fix the problem by padding
coinbase transactions with `OP_0` when necessary to produce valid blocks.
Additionally this commit stops adding `OP_0` padding to the template
`script_sig_prefix` field when `include_dummy_extranonce` is true. This is safe
because non-IPC clients don't use this field, and IPC clients could never set
the option to true, and are expected to add their own nonces in any case.
This also improves documentation about the `script_sig_prefix` field and
`getCoinbaseTx` method.
DEBUG_LOCKORDER was reporting a false positive deadlock with the
cmpctblock fuzz harness when using ImmediateTaskRunner. Since it is
single-threaded, ImmediateTaskRunner callbacks added LockOrders that
could never happen outside of a fuzz test.
First a block would get connected:
* LOCK(mempool.cs)
* BlockConnected (fuzz test runs in same thread)
* LOCK(m_tx_download_mutex)
Then a later iteration of the LIMITED_WHILE would send a TX:
* LOCK(m_tx_download_mutex)
* LOCK(mempool.cs)
causing a false positive deadlock. Normally, the BlockConnected
callback would run in a different thread and no deadlock is reported.
Fix this by launching a thread that runs the callback and is
immediately joined.
The FreeBSD, NetBSD, and OpenBSD build guides state that ZMQ support is compiled in when the package is installed. Since WITH_ZMQ defaults to OFF, update the wording to mention the required CMake option.
On new regtest / signet chains (heights <= 16), createBlock() fails
internally with bad-cb-length. Since mining.capnp does not expose
include_dummy_extranonce, IPC clients can't work around this.
Add a functional test to illustrate this issue and use RPC to
work around it. The next commit introduces a fix.
Needed in a later commit to correctly derive the BIP34 prefix
for heights <= 16.
Add a padding parameter to script_BIP34_coinbase_height() that
controls whether the OP_0 dummy extranonce is appended for
heights <= 16.
Use this helper with padding=False in the IPC mining test's
build_coinbase_test().
Compared to `CreateMuSig2Nonce`, creating a partial signature
has a stronger link to the secret key used, but for consistency
reasons it still makes sense to move all functionality that call
the secp256k1 musig API functions to the `musig.{h,cpp}` module
for consistency.
Can be reviewed via the git option `--color-moved=dimmed-zebra`.
Nonce creation is mainly derived by randomness, and the secret
key merely serves as (optional) additional data for increasing
misuse-resistance, rather than being a central part that would
justify an own CKey method, so move it to the musig.cpp module.
Can be reviewed via the git option `--color-moved=dimmed-zebra`.
The fuzzing quickstart documents the libfuzzer preset with build_fuzz
as the build directory, then mentions libfuzzer-nosan as an alternative
without noting that this preset uses a different binary directory
(build_fuzz_nosan, per CMakePresets.json line 54). A reader following
the quickstart with --preset=libfuzzer-nosan and then running
'cmake --build build_fuzz' as shown will operate against the wrong
(or empty) directory.
Add a clarifying note at the first mention in the Quickstart.
701bc2dc02 contrib: Fix NameError in signet miner gbt() (Torkel Rogstad)
Pull request description:
The logging.warning call referenced `bci["bestblockhash"]`, a variable from the calling scope `do_generate()` that is not available inside the `Generate.gbt()` method. This would crash with a NameError when getblocktemplate returned a template based on an unexpected previous block.
Use the `bestblockhash` parameter that was already being passed in and used correctly in the comparison on the line above.
The bug was introduced in 7b31332370 when the gbt logic was extracted into its own method — the if-condition was updated but the logging call was not.
ACKs for top commit:
kevkevinpal:
ACK [701bc2d](701bc2dc02)
sedited:
ACK 701bc2dc02
Tree-SHA512: 53764db954dd40b68d2f92279fd609732402fdbac4a1838aeee81d366b2b7fbc8da86b07f6f05cee62cb8ecdc514f6e0af1e59c65380eec38cbeaa58ec3c10ed
This refactor makes the ruff.toml easier to read, because it lists the
ignored checks instead of having to enumerate all checks to select in a
large collection of rules.
`CalculateMaximumSignedInputSize()` is passed the outpoint being sized, but that context was not used when estimating the signed input size.
Pass the outpoint through so externally selected inputs are not underestimated.
Co-authored-by: Antoine Poinsot <darosior@protonmail.com>
2424e52836 lint: doc: detail lint tool install methods (will)
5fefa5a654 Don't pin Python patch version (Sjors Provoost)
fd15b55c2e lint: use requirements.txt (will)
5f4d3383da lint: switch to ruff for formatting and linting (will)
a53b81ce4e lint: switch to uv for python management in linter (will)
Pull request description:
Modernise our lint tooling by:
\- Replacing pyenv + pip with [uv](https://docs.astral.sh/uv/) for better Python environment and dependency management
\- Move uv ruff and ty to install via `COPY --from` multi-stage Docker image imports
\- Moving ruff lint rules from hardcoded Rust array (in lint_py.rs) into a top-level ruff.toml
\- Extracting all remaining pip dependencies into dedicated ci/lint/requirements.txt
Extra rationale:
`COPY --from` pulls pre-built binaries from upstream images instead of compiling/downloading at runtime. Containerfile layer optimisations reduce rebuild frequency further.
Pinning tool versions in the dockerfile makes it more excplicit and easier to find.
The tradeoff we make here is that there is no longer a single install script to install tooling on a local machine. However I think this is OK, as it currently only works for `apt`-based OSes anyway, and I don't think running the linter outside of the container is such a valuable use-case as it is with some of the other CI jobs.
ACKs for top commit:
maflcko:
review ACK 2424e52836🗿
sedited:
ACK 2424e52836
Tree-SHA512: 32ef989c1e241cebe5f13da10abd23f6f63306591fd1f81880d688b886082bca17987591dc592c41fbb72278eba57b3cc6e786de7cfa80eb490ab34465d0119b
ac1ccc5bd9 build: Add CTAD feature check (Pol Espinasa)
9f273f1c1c build: Add path to doc recommended versions for CLANG, GCC and MSVC (Pol Espinasa)
Pull request description:
Adds a compiler features check. If failed, it returns a fatal error.
This early stop at configure time will avoid contributors losing time trying to fix compilation errors because of not fitting the requirements.
Example of the output for GCC:
Version requirement satisfied:
```
$ cmake -B build
-- The CXX compiler identification is GNU 13.3.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
...
-- Checking for required C++ features
-- Checking for required C++ features - done
....
```
Compiler feature requirement not satisfied:
```
$ cmake -B build -DCMAKE_C_COMPILER=/usr/bin/gcc-10 -DCMAKE_CXX_COMPILER=/usr/bin/g++-10
-- The CXX compiler identification is GNU 10.5.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++-10 - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Setting build type to "RelWithDebInfo" as none was specified
-- Performing Test CXX_SUPPORTS__WERROR
-- Performing Test CXX_SUPPORTS__WERROR - Success
-- Found SQLite3: /usr/include (found suitable version "3.45.1", minimum required is "3.7.17")
-- Checking for required C++ features
CMake Error at cmake/module/CheckCXXFeatures.cmake:32 (message):
Compiler lacks Class Template Argument Deduction (CTAD) for aggregates.
This C++ feature is required for src/util/overloaded.h.
You are probably using an old compiler version
The recommended compiler versions can be checked in:
- GCC -> doc/dependencies.md#compiler
- Clang -> doc/dependencies.md#compiler
- MSVC -> doc/build-windows-msvc.md
Call Stack (most recent call first):
CMakeLists.txt:198 (check_cxx_features)
-- Configuring incomplete, errors occurred!
```
Example of the output for Clang:
Compiler feature requirement satisfied:
```
$ cmake -B build -DCMAKE_C_COMPILER=clang-20 -DCMAKE_CXX_COMPILER=clang++-20
-- The CXX compiler identification is Clang 20.1.2
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/clang++-20 - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
...
-- Checking for required C++ features
-- Checking for required C++ features - done
...
```
Compiler feature requirement not satisfied:
```
$ cmake -B build -DCMAKE_C_COMPILER=clang-16 -DCMAKE_CXX_COMPILER=clang++-16
-- The CXX compiler identification is Clang 16.0.6
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/clang++-16 - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Setting build type to "RelWithDebInfo" as none was specified
-- Performing Test CXX_SUPPORTS__WERROR
-- Performing Test CXX_SUPPORTS__WERROR - Success
-- Performing Test CXX_SUPPORTS__G3
-- Performing Test CXX_SUPPORTS__G3 - Success
-- Performing Test LINKER_SUPPORTS__G3
-- Performing Test LINKER_SUPPORTS__G3 - Success
-- Performing Test CXX_SUPPORTS__FTRAPV
-- Performing Test CXX_SUPPORTS__FTRAPV - Success
-- Performing Test LINKER_SUPPORTS__FTRAPV
-- Performing Test LINKER_SUPPORTS__FTRAPV - Success
-- Found SQLite3: /usr/include (found suitable version "3.45.1", minimum required is "3.7.17")
-- Checking for required C++ features
CMake Error at cmake/module/CheckCXXFeatures.cmake:32 (message):
Compiler lacks Class Template Argument Deduction (CTAD) for aggregates.
This C++ feature is required for src/util/overloaded.h.
You are probably using an old compiler version
The recommended compiler versions can be checked in:
- GCC -> doc/dependencies.md#compiler
- Clang -> doc/dependencies.md#compiler
- MSVC -> doc/build-windows-msvc.md
Call Stack (most recent call first):
CMakeLists.txt:198 (check_cxx_features)
-- Configuring incomplete, errors occurred!
```
Edit: The first version of the PR was to check hardcoded compiler versions. See previous PR description for context here.
<details>
Adds a compiler minimum version check. If failed, it returns a fatal error.
This early stop at configure time will avoid contributors losing time trying to fix compilation errors because of not fitting the requirements.
Example of the output for GCC:
Version requirement satisfied:
```
sliv3r@sliv3r-tuxedo:~/Documentos/Projectes/BitcoinCore/bitcoin$ cmake -B BUILD
-- The CXX compiler identification is GNU 13.3.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
....
```
Version requirement not satisfied:
```
$ cmake -B build -S . -DCMAKE_C_COMPILER=/usr/bin/gcc-11 -DCMAKE_CXX_COMPILER=/usr/bin/g++-11
-- The CXX compiler identification is GNU 11.5.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++-11 - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at CMakeLists.txt:91 (message):
GCC >= 12.1 required.
-- Configuring incomplete, errors occurred!
```
Example of the output for Clang:
Version requirement satisfied:
```
sliv3r@sliv3r-tuxedo:~/Documentos/Projectes/BitcoinCore/bitcoin$ cmake -B build
-- The CXX compiler identification is Clang 18.1.3
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
...
```
Version requirement not satisfied:
```
$ cmake -B build -DCMAKE_C_COMPILER=clang-16 -DCMAKE_CXX_COMPILER=clang++-16
-- The CXX compiler identification is Clang 16.0.6
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/clang++-16 - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at CMakeLists.txt:86 (message):
Clang >= 17.0 required.
-- Configuring incomplete, errors occurred!
```
<\detail>
ACKs for top commit:
davidgumberg:
crACK ac1ccc5bd9
achow101:
ACK ac1ccc5bd9
vasild:
ACK ac1ccc5bd9
w0xlt:
ACK ac1ccc5bd9
ryanofsky:
Code review ACK ac1ccc5bd9. Very much agree with all the previous comments to not check specific compiler versions, and glad this PR is in better shape now.
Tree-SHA512: 15a12f56016846427ed8273406c3e9a52a6435bf7033c8fef82bb0e6c9fd75d40a62c79d33f9dd6da8717500f21a629da07620f160b3d43368dd7579f773b4bc
f24a7b5f75 doc: recommend script_flags instead of deployments.taproot (Sjors Provoost)
Pull request description:
#26201 removed `taproot` from `getdeploymentinfo` (not yet in a release, slated for v32), which it turns out Lnd relies on: https://github.com/lightningnetwork/lnd/pull/10683
Expand the release doc note to recommend the same solution they used: check for `TAPROOT` in the `script_flags` array. This was added in v31.
ACKs for top commit:
maflcko:
Stakeholder-aligned SLA-compliant production-ready enterprise-grade lgtm ACK f24a7b5f75
sedited:
ACK f24a7b5f75
Tree-SHA512: 5c21300ce3140eb1dd122e007b2f2cfafa3de83db4fb7c699312d6886d4c8fea56d9602390c1931fc640bf9364b2274dde85b38da4957311ddc07721cbc924a7
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
e2b0984f99 wallet: check BDB last page LSN (Lőrinc)
Pull request description:
### Problem
Legacy wallet migration uses the read-only BDB parser to verify that every page LSN is reset before reading records without BDB log files.
The BDB `last_page` metadata field stores the last valid page number, but the parser treated it like a page count and scanned only `0..<last_page`:
e2b0984f99/src/wallet/migrate.cpp (L87)
This skipped the final page, so a database whose last page still depended on BDB logs could be accepted.
### Fix
Scan LSNs through `last_page` inclusively.
ACKs for top commit:
achow101:
ACK e2b0984f99
w0xlt:
ACK e2b0984f99
sedited:
ACK e2b0984f99
Tree-SHA512: 26fade6cdb4747d299b6e620646aa14751cd91fbb7e40ab6e35c1ca796fb589a2340d66108b812611f2924136a8f12c4f911efe6346fffaf04b2d3d288101cda
4defc466a2 cmake: Set `CTEST_NIGHTLY_START_TIME` for CDash Nightly pipelines (Hennadii Stepanov)
Pull request description:
This PR follows up on https://github.com/bitcoin/bitcoin/pull/35222.
According to the [CMake documentation](https://cmake.org/cmake/help/latest/variable/CTEST_NIGHTLY_START_TIME.html) for `CTEST_NIGHTLY_START_TIME`:
> ... this variable must always be set for a nightly build in a dashboard script.
Examples of nightly build reports utilizing this configuration can be found on the [Bitcoin Core CDash board](https://my.cdash.org/index.php?project=bitcoin-core), based on [this commit](98aad4f72f).
ACKs for top commit:
ferminquant:
ACK 4defc466a2
purpleKarrot:
ACK 4defc466a2
Tree-SHA512: 7d3f5dc9f9f1336fc03f565d36750c66bdbae5a3916dabf4ab02c3c2584fc7135a8839e94d7fa08bcbb530913cf0eea31fc402dbdc1c22e207402c991dac044d
9f7a2293c4 depends: Unset `SOURCE_DATE_EPOCH` in `gen_id` script (Hennadii Stepanov)
Pull request description:
When performing Guix builds for `{x86_64,arm64}-apple-darwin` hosts across different commits, all packages in `depends` are rebuilt even if there are no changes in either the `depends` or `contrib/guix` subdirectories.
This occurs because the `SOURCE_DATE_EPOCH` environment variable enables Clang's `-source-date-epoch` option, which then appears in the output of `clang -v -E -xc -o /dev/null - < /dev/null`. For example:
```
$ SOURCE_DATE_EPOCH=1767855465 clang -v -E -xc++ -o /dev/null - < /dev/null
clang version 21.1.7 (Fedora 21.1.7-1.fc43)
Target: x86_64-redhat-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Configuration file: /etc/clang/x86_64-redhat-linux-gnu-clang.cfg
System configuration file directory: /etc/clang/
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/14
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/15
Selected GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/15
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Selected multilib: .;@m64
(in-process)
"/usr/bin/clang-21" -cc1 -triple x86_64-redhat-linux-gnu -E -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name - -mrelocation-model static -mframe-pointer=all -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -funwind-tables=2 -target-cpu x86-64 -tune-cpu generic -debugger-tuning=gdb -fdebug-compilation-dir=/home/hebasto -v -fcoverage-compilation-dir=/home/hebasto -resource-dir /usr/bin/../lib/clang/21 -internal-isystem /usr/bin/../lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15 -internal-isystem /usr/bin/../lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/x86_64-redhat-linux -internal-isystem /usr/bin/../lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/backward -internal-isystem /usr/bin/../lib/clang/21/include -internal-isystem /usr/local/include -internal-isystem /usr/bin/../lib/gcc/x86_64-redhat-linux/15/../../../../x86_64-redhat-linux/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -source-date-epoch 1767855465 -fdeprecated-macro -ferror-limit 19 -fmessage-length=180 -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fcxx-exceptions -fexceptions -fcolor-diagnostics -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -o /dev/null -x c++ -
clang -cc1 version 21.1.7 based upon LLVM 21.1.7 default target x86_64-redhat-linux-gnu
ignoring nonexistent directory "/usr/bin/../lib/gcc/x86_64-redhat-linux/15/../../../../x86_64-redhat-linux/include"
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
/usr/bin/../lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15
/usr/bin/../lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/x86_64-redhat-linux
/usr/bin/../lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/backward
/usr/bin/../lib/clang/21/include
/usr/local/include
/usr/include
End of search list.
```
As a result, each package id differs between builds, which causes the script to treat the toolchain as changed and triggers unnecessary rebuilds.
This PR resolves this issue by [clobbering](https://github.com/bitcoin/bitcoin/pull/34228#issuecomment-4387135450) the `SOURCE_DATE_EPOCH` value in the `gen_id` script.
---
Suggested testing scenario:
```
$ env HOSTS=arm64-apple-darwin ./contrib/guix/guix-build
$ git commit --allow-empty -m "Trigger rebuild"
$ env HOSTS=arm64-apple-darwin ./contrib/guix/guix-build
````
The last command will rebuild depends on the master branch, but will successfully use the cached built packages on this PR.
ACKs for top commit:
maflcko:
lgtm ACK 9f7a2293c4
fanquake:
ACK 9f7a2293c4
Tree-SHA512: d5fa90100edfd88024ad949a9d75c8af274a054c0926ae4ddea6ecf1c38fae833670b360c56c740a7565cf393a92597823749fe75c85a27ae1ebebead4093853
086894098e cmake: add CTestConfig.cmake (will)
Pull request description:
Add a `CTestConfig.cmake` file with the CDash submission URL for Bitcoin Core at the project root.
This lets developers use CTest’s built-in dashboard support to upload local configure, build, and test results to CDash. With this a developer can use their own CTest dashboard script or manual `ctest` workflow and upload results to a collection point.
This is useful for sharing reproducible build/test results from local machines, CI experiments, and platform-specific debugging. CDash keeps the configure output, build warnings/errors, and test results grouped under a single dashboard submission, making it easier to inspect failures and compare behavior across environments.
The file only defines the CDash upload location. It does not change the default build or test behavior.
**WARNING** uploaded test results may also include (potentially) identifiable information, such as hostnames, usernames, especially if you attach notes which are located within you rhome directory, as the full file path is printed.
ACKs for top commit:
purpleKarrot:
ACK 086894098e
maflcko:
lgtm ACK 086894098e
hebasto:
ACK 086894098e.
Tree-SHA512: 5d0e2e174773de3f56af80252a62330fc313f394e1a868ac0d2a83c6610c2f734f6e6938bfced710879c00f64f09e17030da594b24940d77c984e46698d37647
fad61896e8 ci: Move --usecli --extended from i386 task to alpine task (MarcoFalke)
Pull request description:
The i386 task is getting increasingly tedious to maintain:
* It is using deprecated (disabled by default) syscalls, see commit 999e9dbfb4
* Running it on (e.g.) aarch64 is slow, due to the additional qemu overhead
It is mostly kept to have some more 32-bit coverage for https://github.com/bitcoin/bitcoin/issues/32375. But maybe in 5 or 10 years, it can be removed .... ?
So for now, try to reduce the config it runs by moving it out to longer-term tasks.
Specifically, move `TEST_RUNNER_EXTRA="--v2transport --usecli --extended"` to the native Alpine task, which should exist long-term, runs natively, and also has a debug build enabled.
ACKs for top commit:
polespinasa:
ACK fad61896e8
sedited:
ACK fad61896e8
Tree-SHA512: a406347aa06de7edbfe8c7d3d082983fa54f5c266c1b2344102cd6cd59dc5b9f46da5e15edfaa7655edf1a2ff8d70da18bae4c8297dfb0cf2c783098d8a46c30
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
ac58e6c53c test: fix P2SH output in coins cache fuzz (Lőrinc)
Pull request description:
### Problem
`coinscache_sim` manually constructs a 23-byte P2SH `scriptPubKey`, but placed `OP_EQUAL` at byte index 12.
That index is inside the 20-byte script hash payload, so the constructed script did not match the standard P2SH layout:
fa2670bd4b/src/script/script.cpp (L223-L230)
### Fix
Place `OP_EQUAL` after `OP_HASH160`, the 20-byte push opcode, and the 20-byte script hash.
Also remove a stray trailing comment terminator in the same fuzz target.
ACKs for top commit:
Crypt-iQ:
crACK ac58e6c53c
brunoerg:
ACK ac58e6c53c
sedited:
ACK ac58e6c53c
Tree-SHA512: 1f909dd25d9df87a923e6496145da0ada2b1fa6511b61fb2d203db4c7724f2341c898862a15e7051b952bca834e6654c70fba64a7bf223bfd6d399b3b5d9e59b
fa864b937e refactor: [rpc] Remove confusing and brittle integral casts (take 3) (MarcoFalke)
Pull request description:
This one cast is harmless, but confusing. Also, in the past they have been brittle to the extend of triggering bugs. See commit 44afed4cd9. So remove this one recently introduced one.
ACKs for top commit:
fjahr:
ACK fa864b937e
stickies-v:
ACK fa864b937e
sedited:
ACK fa864b937e
Tree-SHA512: 78407b97964c144f4d94cd445af4f57e29e460912ae9178898224d71f3d5237e5db88a981f7636193a6b7a22be8ad4fd833eb0efa9507284c10f87d9da0ec81b
The BDB metadata field `last_page` stores the last valid page number, not the number of pages.
The read-only wallet migration parser currently checks reset LSNs with a half-open loop, so it skips the final page and may accept a database whose last page still depends on BDB log files.
