5d784bebaf clang-tidy: Disable `ArrayBound` check in src/ipc and src/test (Hennadii Stepanov)
5efdb0ef30 ci: Update Clang in "tidy" job (Hennadii Stepanov)
Pull request description:
This PR:
1. Updates to [IWYU 0.25](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.25), which is compatible with Clang 21.
2. Fixes new "modernize-use-default-member-init" warnings. The warning in `interpreter.cpp` is a [false positive](https://github.com/llvm/llvm-project/issues/160394), so it has been suppressed.
ACKs for top commit:
maflcko:
review ACK 5d784bebaf🎒
ryanofsky:
Code review ACK 5d784bebaf, just adding clang version comment since last review.
Tree-SHA512: a1d853675ec064170ee0f1cd16be6a900676588d4a1e7b5def8733933b140ba1a9520ec6f6a42bf7638b2ff7cf2fe4d5866d407f68b677b49d2bd68ff345f735
In `QSortFilterProxyModel`, `invalidateFilter()` is scheduled for
deprecation in Qt 6.13.
`beginFilterChange()` was introduced in Qt 6.9.
`endFilterChange()` was introduced in Qt 6.10.
4e352efa2c qt: add createwallet, createwalletdescriptor, and migratewallet to history filter (WakeTrainDev)
Pull request description:
Added `createwallet`, `createwalletdescriptor` and `migratewallet` RPC commands to the Qt console history filter since they may include passphrases or other sensitive data that should not be stored in command history.
ACKs for top commit:
pablomartin4btc:
utACK 4e352efa2c
hebasto:
ACK 4e352efa2c.
Tree-SHA512: dc6a12b95173b1e476d483381df3d74add88a1e225c90b1b60db59eab6d504a2496b66890ccec28c691745e405a3053d72afda9d80ae96a703f12cd256e4ebd6
53e4951a5b Switch to ANSI Windows API in `fsbridge::fopen()` function (Hennadii Stepanov)
dbe770d921 Switch to ANSI Windows API in `Win32ErrorString()` function (Hennadii Stepanov)
06d0be4e22 Remove no longer necessary `WinCmdLineArgs` class (Hennadii Stepanov)
f366408492 cmake: Set process code page to UTF-8 on Windows (Hennadii Stepanov)
dccbb17806 Set minimum supported Windows version to 1903 (May 2019 Update) (Hennadii Stepanov)
Pull request description:
The main goal is to remove [deprecated](https://github.com/bitcoin/bitcoin/issues/32361) code (removed in C++26).
This PR employs Microsoft's modern [approach](https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page) to handling UTF-8:
> Until recently, Windows has emphasized "Unicode" -W variants over -A APIs. However, recent releases have used the ANSI code page and -A APIs as a means to introduce UTF-8 support to apps. If the ANSI code page is configured for UTF-8, then -A APIs typically operate in UTF-8. This model has the benefit of supporting existing code built with -A APIs without any code changes.
TODO:
- [x] Handle application manifests properly when building with MSVC.
- [x] Bump the minimum supported Windows version to 1903 (May 2019 Update).
- [x] Remove all remaining use cases of the deprecated `std:wstring_convert`.
- The instance in `subprocess.h` will be addressed in a follow-up PR, as additional tests are likely needed.
- The usage in `common/system.cpp` is handled in https://github.com/bitcoin/bitcoin/pull/32566.
Resolves partially https://github.com/bitcoin/bitcoin/issues/32361.
ACKs for top commit:
laanwj:
re-ACK 53e4951a5b
hodlinator:
re-ACK 53e4951a5b
davidgumberg:
untested crACK 53e4951a5b
Tree-SHA512: 0dbe9badca8b979ac2b4814fea6e4a7e53c423a1c96cb76ce894253137d3640a87631a5b22b9645e8f0c2a36a107122eb19ed8e92978c17384ffa8b9ab9993b5
This change updates to IWYU 0.25, which is compatible with Clang 21.
Fixes new "modernize-use-default-member-init" warnings.
The warning in `interpreter.cpp` is a false positive, so it has been
suppressed.
As reported by hebasto in https://github.com/bitcoin/bitcoin/issues/33545,
newer libc++ versions implementing https://wg21.link/lwg3430 will no longer
implicitly convert `fs::path` objects to `std::filesystem::path` objects when
constructing `std::ifstream` and `std::ofstream` types.
This is not a problem in Unix systems since `fs::path` objects use
`std::string` as their native string type, but it causes compile errors on
Windows which use `std::wstring` as their string type, since `fstream`s can't
be constructed from `wstring`s.
Fix the windows libc++ compile errors by adding a new `fs::path::std_path()`
method and using it construct `fstream`s more portably.
Additionally, delete `fs::path`'s implicit `native_string` conversion so these
errors will not go undetected in the future, even though there is not currently
a CI job testing Windows libc++ builds.
6a371b70c8 gui: Avoid pathological QT text/markdown behavior... (David Gumberg)
Pull request description:
...during text selection by only setting plaintext mime data.
Fixes the OOM described in #887.
The issue is related to the construction of the [`text/markdown`](b617d11765/src/widgets/widgets/qwidgettextcontrol.cpp (L3539)) MIME data for the selection. Using the `heaptrack` utility, I observed that nearly all of the allocations when reproducing happen in [`QTextMarkdownWriter::writeFrame`](b617d11765/src/gui/text/qtextmarkdownwriter.cpp (L95)). I am not 100% sure what is causing this issue in QT's conversion of our HTML to markdown; I have tried changing the [HTML tags](689a321976/src/qt/rpcconsole.cpp (L916-L924)) (e.g. using `<p></p`> and `<ul><li></li></ul>` in place of tables) used in our `rpcconsole` messages, but the issue recurs.
The solution applied here is to override `createMimeDataFromSelection()` to avoid construction of the (likely never-used anyways) `text/markdown` mime data, and only set plaintext mime data in the clipboard.
ACKs for top commit:
hebasto:
ACK 6a371b70c8.
Tree-SHA512: 3edc4da47e6dbe939f27664d2265376938eed4f83ded3706e4b73677eac5c9a4ba8819f241428b45a08e8834982ee7759ee096afd090586db3b523d0ccbbbf73
be776a1443 wallet: Remove isminetype (Ava Chow)
009a69a616 wallet: Remove ISMINE_USED (Ava Chow)
6a7aa01574 wallet: Remove COutput::spendable and AvailableCoinsListUnspent (Ava Chow)
620abe985e interfaces, gui: Remove is_mine output parameter from getAddress (Ava Chow)
Pull request description:
The remaining isminetypes are `ISMINE_SPENDABLE` and `ISMINE_USED`.
`ISMINE_USED` is only used as a filter for caching balances and is never actually returned from `IsMine`. Since we do still want this behavior, This PR changes the caching to utilize bools and explicit members variables to account for the avoid_reuse case. This allows us to remove `ISMINE_USED`.
`ISMINE_SPENDABLE` and `ISMINE_NO` are the only things that are returned by `IsMine`. This is a bool, so it can be replaced as such.
After removing `ISMINE_USED` and `ISMINE_SPENDABLE`, we are able to remove isminetypes altogether.
ACKs for top commit:
murchandamus:
ACK be776a1443
fjahr:
reACK be776a1443
davidgumberg:
crACK be776a1443
enirox001:
re-ACK be776a1
jlest01:
reACK be776a1443
Tree-SHA512: 689759f6a6ba20a1ae988b0c3abacb15424844f29a1ec2fcb2d1ca9d87b44ae68313e8f61d6fd310281b681144f0ade67e90fcfab807e982b52ed99441d9c987
Since the only remaining isminetypes are ISMINE_NO and ISMINE_SPENDABLE,
this enum is now just a bool and can be removed. IsMine is changed to
return a bool and any usage of isminetypes and isminefilters are changed
to be the remaining ISMINE_SPENDABLE case.
This change avoids hardcoding the "debug.log" filename, ensuring
compatibility with custom filenames provided via the `-debuglogfile`
command-line option.
Using `file(GLOB)` in the generates step is discouraged because the
globbing result may be out of date when the target is built.
Performing the globbing in a script that is executed as the build
target means the result is always reproducable and the overhead
of globbing is only paid when used.
As a follow up, the dependency on `sed` may be removed by performing
the replacement with cmake. Also, the logic from extract_strings_qt.py
can be migrated to cmake.
de0675f9de refactor: Move `transaction_identifier.h` to primitives (marcofleon)
6f068f65de Remove implicit uint256 conversion and comparison (marcofleon)
9c24cda72e refactor: Convert remaining instances from uint256 to Txid (marcofleon)
d2ecd6815d policy, refactor: Convert uint256 to Txid (marcofleon)
f6c0d1d231 mempool, refactor: Convert uint256 to Txid (marcofleon)
aeb0f78330 refactor: Convert `mini_miner` from uint256 to Txid (marcofleon)
326f244724 refactor: Convert RPCs and `merkleblock` from uint256 to Txid (marcofleon)
49b3d3a92a Clean up `FindTxForGetData` (marcofleon)
Pull request description:
This is the final leg of the [type safety refactor](https://github.com/bitcoin/bitcoin/pull/32189).
All of these changes are straightforward `uint256` --> `Txid` along with any necessary explicit conversions. Also, `transaction_identifier.h` is moved to primitives in the last commit, as `Txid` and `Wtxid` become fundamental types after this PR.
ACKs for top commit:
stickies-v:
re-ACK de0675f9de, no changes since a20724d926d5844168c6a13fa8293df8c8927efe except address review nits.
janb84:
re ACK de0675f9de
dergoegge:
re-ACK de0675f9de
theStack:
Code-review ACK de0675f9de
Tree-SHA512: 2413160fca7ab146a8d79d18ce3afcf7384cacc73c513d41928904aa453b4dd7a350064cee71e9c5d015da5904c7c81ac17603e50a47441ebc5b0c653235dd08
Moves the file from `src/util` to `src/primitives`. Now that the
refactor is complete, Txid and Wtxid are fundamental types, so it
makes sense for them to reside in `src/primitives`.
656e16aa5e qt: Update the `src/qt/locale/bitcoin_en.xlf` translation source file (Hennadii Stepanov)
a0eaa44925 Fix typos (Hennadii Stepanov)
8d4aaaec49 Update Transifex slug for 30.x (Hennadii Stepanov)
Pull request description:
This PR follows our [Release Process](53a996f122/doc/release-process.md).
It is required to open Transifex translations for v30.0, as scheduled in https://github.com/bitcoin/bitcoin/issues/32275.
For reference, see the previous similar PR: https://github.com/bitcoin/bitcoin/pull/31809.
**Note for reviewers:**
To reproduce the diff in the last commit, run:
```
cmake --preset dev-mode
cmake --build build_dev_mode --target translate
```
ACKs for top commit:
laanwj:
Code review ACK 656e16aa5e
stickies-v:
ACK 656e16aa5e , was able to reproduce
Tree-SHA512: 403b534329755079584fcdf98b696e3e75952dfc8d069f305843dbfa85de95f6816ee1d5dfc9b553c7c7f52cc296cb8d3cb03207051d26e0e76ff30d377f49e4
f49840dd90 doc: Fix typo in files.md (Ryan Ofsky)
f5cf0b1ccc bitcoin wrapper: improve help output (Ryan Ofsky)
c810b168b8 doc: Add description of installed files to files.md (Ryan Ofsky)
94ffd01a02 doc: Add release notes describing libexec/ binaries (Ryan Ofsky)
cd97905ebc 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 f49840dd90
Sjors:
re-ACK f49840dd90
achow101:
ACK f49840dd90
janb84:
re ACK f49840dd90
BrandonOdiwuor:
Tested ACK f49840dd90
hodlinator:
re-ACK f49840dd90
willcl-ark:
utACK f49840dd90
Tree-SHA512: 858a2e1a53db11ee3c5c759bfdeea566f242b9ce5e8a898fa435222e41662b8184577c0dc2c4c058294b4de41d8cb3ba3e5d24c748c280efa4a3f84e3ec4344d
The translations for the following languages, which appear to be the
result of a mistake or an act of vandalism, have been discarded:
- Greek (el)
- Vietnamese (vi)
This change offers a few advantages, such as:
- a more readable and cleaner `ts_files.cmake` (see the next commit);
- a scoped `ts_files` variable;
- improved code locality;
- no need to adjust the location of the resulting `*.qm` files.
For some reason, building x86_64-w64-mingw32 on x86_64 and aarch64
results in a single instruction difference which can be traced down to
prevector.h:174. The ultimate caller of this is the copy constructor for
a prevector that ends up being called by std::vector::push_back in
walletmodel.cpp:183. By replacing the push_back with an emplace_back,
somehow this non-determinism goes away.
fcfd3db563 remove RPCTimerInterface and RPCRunLater (Matthew Zipkin)
8a1765795f use WalletContext scheduler for walletpassphrase callback (Matthew Zipkin)
Pull request description:
This removes the dependency on libevent for events scheduled by RPC commands, like re-locking a wallet some time after decryption with walletpassphrase. Since walletpassphrase is currently the only RPC that does this, `RPCRunLater`, `RPCTimerInterface` and all related methods are left unused, and deleted in the second commit. Any future RPC that needs to execute a callback in the future can follow the pattern in this PR and just use a scheduler from node or wallet context.
This is an alternative approach to #32796, described in https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3014309449
ACKs for top commit:
fjahr:
Code Review ACK fcfd3db563
achow101:
ACK fcfd3db563
furszy:
ACK fcfd3db563
Tree-SHA512: 04f5e9c3f73f598c3d41d6e35bb59c64c7b93b03ad9fce3c40901733147ce7764f41f475fef1527d44af18f722759996a31ca83b48cb52153795d5022fecfd14
67dc7523f3 cmake, test: Disable tests instead of ignoring them (Hennadii Stepanov)
bb9157db5d cmake, refactor: Switch to `Python3::Interpreter` imported target (Hennadii Stepanov)
Pull request description:
This PR:
1. Switches to a modern CMake approach by using the `Python3::Interpreter` imported target, which is more robust than using variables.
2. Disables the `util_rpcauth_test` test explicitly instead of silently ignoring it.
A build and test log for the case when Python is unavailable is provided below:
```
$ cmake -B build
$ cmake --build build -j 16
$ ctest --test-dir build -j $(nproc) -R "^util"
Internal ctest changing into directory: /bitcoin/build
Test project /bitcoin/build
Start 115: util_tests
Start 117: util_trace_tests
Start 114: util_string_tests
Start 116: util_threadnames_tests
Start 1: util_rpcauth_test
1/5 Test #1: util_rpcauth_test ................***Not Run (Disabled) 0.00 sec
2/5 Test #114: util_string_tests ................ Passed 0.11 sec
3/5 Test #117: util_trace_tests ................. Passed 0.11 sec
4/5 Test #116: util_threadnames_tests ........... Passed 0.11 sec
5/5 Test #115: util_tests ....................... Passed 0.13 sec
100% tests passed, 0 tests failed out of 4
Total Test time (real) = 0.13 sec
The following tests did not run:
1 - util_rpcauth_test (Disabled)
```
ACKs for top commit:
purpleKarrot:
ACK 67dc7523f3
janb84:
tACK 67dc7523f3
Tree-SHA512: 5fc7ebe31ac03f4b8a53ecfcfc1cace0f647a1d2c989651988edae96bdfbbe2dee171714e57cb028e65ead1bb40806a82d9821746451dbf005538601fd33ea88
b789907346 wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet (furszy)
e86d71b749 wallet: refactor, dedup wallet re-loading code (furszy)
1de423e0a0 wallet: introduce method to return all db created files (furszy)
d04f6a97ba refactor: remove sqlite dir path back-and-forth conversion (furszy)
Pull request description:
Currently, the migration process creates a brand-new descriptor wallet with no
connection to the user's legacy wallet when the legacy wallet lacks key material
and contains only watch-only scripts. This behavior is not aligned with user
expectations. If the legacy wallet contains only watch-only scripts, the migration
process should only generate a watch-only wallet instead.
TODO List:
* Explain that `migratewallet` renames the watch-only after migration, and
also that the wallet will not have keys enabled.
ACKs for top commit:
achow101:
ACK b789907346
pablomartin4btc:
tACK b789907346
rkrux:
LGTM ACK b789907346
Tree-SHA512: 1d583ac4b206fb477e9727daf4b5ad9c3e18b12d40e1ab4a61e8565da44c3d0327c892b51cf47b4894405d122e414cefb6b6366c357e02a74a7ca96e06762d83
173394d951 depends: Build `qt` package for FreeBSD hosts (Hennadii Stepanov)
Pull request description:
This PR continues the work started in https://github.com/bitcoin/bitcoin/pull/23948.
Here is an excerpt from the log:
```
$ ./build/bin/bitcoin-qt -printtoconsole
2025-06-12T01:06:56Z Bitcoin Core version v29.99.0-15de25ba2a28 (release build)
2025-06-12T01:06:56Z Qt 6.7.3 (static), plugin=xcb
2025-06-12T01:06:56Z Static plugins:
2025-06-12T01:06:56Z QMinimalIntegrationPlugin, version 395008
2025-06-12T01:06:56Z QXcbIntegrationPlugin, version 395008
2025-06-12T01:06:56Z Style: fusion / QFusionStyle
2025-06-12T01:06:56Z System: FreeBSD 14.3-RELEASE, x86_64-little_endian-lp64
```
And here are the screenshots:
ACKs for top commit:
vasild:
ACK 173394d951
Tree-SHA512: 42a0bd11e4ef1a23efcfe6c4ab179dc667a076e65060891ce8358b3fe78de4e3ea33f975387d4236cc2ac620e2935b0a29c278065a47f038c66658106bf36755
fa9ca13f35 refactor: Sort includes of touched source files (MarcoFalke)
facb152697 scripted-diff: Bump copyright headers after include changes (MarcoFalke)
fae71d30f7 clang-tidy: Apply modernize-deprecated-headers (MarcoFalke)
Pull request description:
Bitcoin Core is written in C++, so it is confusing to sometimes use the deprecated C headers (with the `.h` extension). For example, it is less clear whether `string.h` refers to the file in this repo or the cstring stdlib header (https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121492797).
The check is currently disabled for headers, to exclude subtree headers.
ACKs for top commit:
l0rinc:
ACK fa9ca13f35
achow101:
ACK fa9ca13f35
janb84:
ACK fa9ca13f35
stickies-v:
ACK fa9ca13f35
Tree-SHA512: 6639608308c598d612e24435aa519afe92d71b955874b87e527245291fb874b67f3ab95d3a0a5125c6adce5eb41c0d62f6ca488fbbfd60a94f2063d734173f4d
Historically, the headers have been bumped some time after a file has
been touched. Do it now to avoid having to touch them again in the
future for that reason.
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended 's;( 20[0-2][0-9])(-20[0-2][0-9])? The Bitcoin Core developers;\1-present The Bitcoin Core developers;g' $( git show --pretty="" --name-only HEAD~0 )
-END VERIFY SCRIPT-
This can be reproduced according to the developer notes with something
like
( cd ./src/ && ../contrib/devtools/run-clang-tidy.py -p ../bld-cmake -fix -j $(nproc) )
Also, the header related changes were done manually.
