Keep the repeated embedded address documentation in one helper so the OpenRPC metadata and help text stay consistent without duplicating the same field list.
Cover the new RPC with a lightweight functional test that verifies the result is JSON serializable and exposes the expected top-level OpenRPC document fields.
Expose the generated OpenRPC document through RPC so clients can inspect the interface supported by the running node. The command can optionally include hidden RPC commands and arguments for complete code generation and regtest coverage.
RPCResult::Type::ANY triggers NONFATAL_UNREACHABLE() in ToSections(),
which crashes the help() RPC when a command uses Type::ANY in a
nested result field.
Previously this was never hit because Type::ANY was only used as a
top-level alternate result type, filtered out before ToSections() is
called.
getopenrpcinfo() will use this result type, so render it like other
types allowing it to be used in nested result definitions like schema.
After removing the last CRPCCommand pointer for a given name,
erase the now-empty vector from mapCommands. Without this,
listCommands() returns the name of a fully removed command
because it iterates mapCommands keys unconditionally.
For example, when unloading the wallet the RPCs are deregistered, and
this prevents getopenrpcinfo from returning non-existant RPCs.
735b25519a support: clamp RLIMIT_MEMLOCK to size_t (Sjors Provoost)
8ab4b9fc85 init: clamp fd limits to int (Sjors Provoost)
4afbabdcef Fix startup failure with RLIM_INFINITY fd limits (Sjors Provoost)
Pull request description:
When setting the fd limit to unlimited, the node fails to start:
```sh
ulimit -n unlimited
build/bin/bitcoind
Error: Not enough file descriptors available. -1 available, 160 required.
```
This was caused by `RaiseFileDescriptorLimit()` (introduced in #2568) casting `limitFD.rlim_cur` to `int`, which for `RLIM_INFINITY` overflows to `-1`. Fix it by returning `std::numeric_limits<int>::max()` instead.
Some platforms implement `RLIM_INFINITY` as the maximum uint64, others as int64 (-1). So simply changing the return type to `uint64_t` wouldn't work.
Similarly, though unlikely to actually happen:
```sh
ulimit -n 214748364
build/bin/bitcoind
Error: Not enough file descriptors available. -2147483648 available, 160 required.
```
The second commit expands the fix by clamping all values above `std::numeric_limits<int>::max()` instead of letting them overflow.
This PR also expands `test/functional/feature_init.py` to cover these, using `resource.setrlimit`. The check is skipped on environments with a hard limit below infinity (or that don't have the Python [Resource module](https://docs.python.org/3/library/resource.html)).
macOS by default has a hard limit of infinity, but on e.g. Ubuntu the default hard limit is 524288.
The third commit applies a similar fix to `PosixLockedPageAllocator::GetLimit()` for 32-bit systems, but without a test.
ACKs for top commit:
winterrdog:
Re-ACK 735b25519a
achow101:
ACK 735b25519a
sedited:
Re-ACK 735b25519a
pinheadmz:
ACK 735b25519a
Tree-SHA512: 0ce0292ecd61456bdec6943b06cbb9ecfc5180ee6dce850f8496ef54af22c1fae6ea473085202f5ba6f72e4dc51a29247620c9a0eae31e96658adc77b293129f
2447385f47 rpc: remove unused RPCResult::Type::ELISION (satsfy (Renato Britto))
7a85118005 rpc: expand decodepsbt output script with explicit fields (satsfy (Renato Britto))
88e2a6ae89 rpc: expand getaddressinfo embedded with explicit fields (Renato Britto)
a9f9e7d17e rpc: extract fee estimate result helpers (Renato Britto)
8a615a8800 rpc: extract ListSinceBlockTxFields() helper (Renato Britto)
372ac283ac rpc: extend TxDoc() for getblock verbosity 2/3 (Renato Britto)
0380a1c46b rpc: extend TxDoc() for getrawtransaction verbosity 2 (Renato Britto)
44fc3a290d rpc: introduce HelpElision variant and ElideGroup helper (Renato Britto)
Pull request description:
Partially addresses #29912. Motivated by #34683, which exports OpenRPC from existing `RPCHelpMan` metadata. [Sample OpenRPC](https://gist.github.com/natobritto/8c4a1da04968d2325082ac4bca7d2408).
Some RPC help definitions rely on `RPCResult::Type::ELISION` entries whose structure is only described in prose. This keeps human-readable help concise, but leaves parts of the result layout implicit and prevents tools from deriving complete machine-readable schemas from `RPCHelpMan` metadata.
This PR replaces ELISION-based reuse with shared structured definitions, so result layouts are represented directly in metadata rather than only in text. At the same time, human-readable help remains compact via explicit help-rendering elision using `HelpElision`, so previously elided sections stay abbreviated without losing schema completeness.
Affected RPCs: `getrawtransaction`, `getblock`, `listsinceblock`, `estimaterawfee`, `getaddressinfo`.
RPC return values are unchanged. Human-readable help remains compact, while structured result metadata becomes explicit enough to derive complete machine-readable schemas.
A related `RPCResult::Type::ELISION` use in `importdescriptors` was split out into the follow-up PR #34867 because it changes the generated help output, per [this review comment](https://github.com/bitcoin/bitcoin/pull/34764#discussion_r2960236111).
Changes:
- Introduce `HelpElision` (`NONE`, `START`, `SKIP`) and `ElideGroup()`, replacing the tri-state `print_elision`
- Add an `RPCResult` copy-with-replacement-options constructor to support applying elision while keeping `m_opts` const
- Extend `TxDoc()` / `TxDocOptions` to support reusable transaction layouts with optional `prevout`, `fee`, `hex`, and elision behavior
- Replace ELISION-based reuse in `getrawtransaction` and `getblock` with explicit structured definitions
- Factor shared result layouts into `GetBlockFields()`, `ListSinceBlockTxFields()`, `FeeRateBucketDoc()`, `GetAddressInfoEmbeddedFields()` and `FeeEstimateHorizonDoc()`
- Expand `listsinceblock.removed`, `estimaterawfee` horizons/buckets and `getaddressinfo.embedded` into explicit metadata while preserving concise help output
ACKs for top commit:
achow101:
ACK 2447385f47
w0xlt:
reACK 2447385f47
janb84:
re ACK 2447385f47
willcl-ark:
ACK 2447385f47
Tree-SHA512: 8dc03c45c388ebdb4f8f1613af2576fc127a8d5425efe518cd0d0ed2439a38b2ed1236413471672c85f52ee22c4a17677c18fd4689bc6570496fc3af4cd4112f
2818a171c0 test: add abortscan unit test (Pol Espinasa)
bc30e95163 wallet: move fAbortRescan reset into WalletRescanReserver reserve() (Pol Espinasa)
Pull request description:
Follow-up of https://github.com/bitcoin/bitcoin/pull/35179
For extra context refer to the conversations https://github.com/bitcoin/bitcoin/pull/35179#discussion_r3378136210 and comments bellow it.
Long story short: currently `ScanForWalletTransactions()` resets the value of `fAbortRescan` before starting the rescan loop. This can cause a race condition where some function (e.g. `importdescriptors`) starts a rescan and at the same time the user aborts it manually. Could happen that the `abortrescan` call returns True (success) but the rescan continues running as the value is overwritten.
This PR fixes this by resetting the value of `fAbortRescan` at the very beginning, when the wallet rescan is reserved, removing the race condition. Also adds a test for it.
ACKs for top commit:
achow101:
ACK 2818a171c0
w0xlt:
ACK 2818a171c0
nebula-21:
ACK 2818a171c0
pablomartin4btc:
ACK 2818a17
pinheadmz:
ACK 2818a171c0
Tree-SHA512: 5b64b9a16a209dd145ccf7b2d7c3a7205038b3b214b02c3d9f1cbda75a6a1c9a41ad5c42cbeda43af6141793965613c47e44300d758149be9bfdf458e85a5520
0654511e1b util: Check write failures before renaming settings.json (Shrey)
Pull request description:
This PR Fixes#35373.
The error message was updated from: "This is probably caused by disk corruption or a crash" to: "This is probably caused by a full disk, disk corruption or a crash"
The following files were updated:
1. src/common/settings.cpp -- Updated the main string formatting for the parse failure.
2. src/test/settings_tests.cpp -- Updated the exact string expectation in the C++ unit test.
3. test/functional/feature_settings.py -- Updated the expected error message in the Python test suite.
Configured the Bitcoin Core project natively on Windows using Visual Studio (cmake --preset vs2026-static -DBUILD_GUI=OFF) and it compiled with 100% of the C++ tests passed successfully (347 out of 347).
ACKs for top commit:
maflcko:
review ACK 0654511e1b💧
winterrdog:
ACK 0654511e1b
sedited:
ACK 0654511e1b
ryanofsky:
Code review ACK 0654511e1b with commit message and error message improved since last review
Tree-SHA512: d7b49860f2081a5a9d1d44917b0cf372a77f10cf21773c9b3291871c788a122ec5dde063fdf9e1052cf45b8d2667e15585b952e9f037e84f98fb41546bdd1fa9
de92208c2b migrate: Handle HD chains that have identical seeds but different IDs (Ava Chow)
Pull request description:
The seed ID is calculated from a pubkey produced by treating the seed as a private key. This calculation includes a pubkey compression parameter, even thought that compression is completely irrelevant for the usage of the seed as a BIP 32 seed. Thus migration should detect if a seed has been used multiple times by checking if the computed master key was already processed.
The spkm_migration fuzzer needs to have it's added descriptors accounting updated for this fix.
It should not be possible for users to actually run into this problem as all HD chains use seeds with the pubkey compression option set.
Fixes#35434
ACKs for top commit:
kevkevinpal:
crACK [de92208](de92208c2b)
marcofleon:
crACK de92208c2b
rkrux:
code review ACK de92208
Tree-SHA512: c420a24722fd6a94bf6656f195bad3432ba54c38b3c49a02750577281d0864988fd6d44cd9594b57cfaf33061a1e250e21378e3637b4e9a45f2d7aad6045884d
f963f2b675 argsman: allow duplicate registration between HIDDEN and other categories (Pablo Martin)
Pull request description:
Follow-up to #35470.
The assertion added in #35470 to prevent duplicate option registration across categories was too strict, it also fired when an option was registered in `OptionsCategory::HIDDEN` and then again in a real category (or vice versa).
That is intentional behavior introduced in #13441: options unavailable in a given binary (e.g. `GUI` args in `bitcoind`) are pre-registered as hidden so shared `bitcoin.conf` files don't fail. In `bitcoin-qt`, `SetupServerArgs` registers `GUI` args as hidden, then `SetupUIArgs` registers them properly under `OptionsCategory::GUI`, triggering the assertion and crashing on startup (e.g. `bitcoin-qt` crashes now that #35470 has been merged into `master`).
The fix relaxes the assertion to exclude `HIDDEN` from the cross-category duplicate check, preserving the original intent of #13441 while still catching unintentional duplicates between real categories.
<details>
<summary>Alternative approach considered</summary>
An alternative fix would have been to make `AddHiddenArgs` skip args already registered in any category:
```cpp
void ArgsManager::AddHiddenArgs(const std::vector<std::string>& names)
{
for (const std::string& name : names) {
size_t eq_index = name.find('=');
std::string arg_name = name.substr(0, eq_index == std::string::npos ? name.size() : eq_index);
LOCK(cs_args);
bool already_registered = std::ranges::any_of(m_available_args, [&](const auto& arg_map) {
return arg_map.second.contains(arg_name);
});
if (!already_registered) {
AddArg(name, "", ArgsManager::ALLOW_ANY, OptionsCategory::HIDDEN);
}
}
}
```
This would fix the crash but obscures the intent — silently skipping registrations in `AddHiddenArgs` makes it harder to reason about what's registered. The chosen approach of relaxing the assertion specifically for `HIDDEN` is more explicit about why the duplicate is allowed.
</details>
ACKs for top commit:
ryanofsky:
Code review ACK f963f2b675 and confirmed this fixes the problem.
sedited:
ACK f963f2b675
Tree-SHA512: a7606d66c99f3d9ff485abcd7c6e3266ce42a3ec716863a6c756a752c6c92a3c0e5a969d2c3646ade9f13032a42fc364b0bbfa9692e723ae61327284ed1caa2e
The assertion added in #35470 to prevent duplicate option registration
across categories was too strict — it also fired when an option was
registered in OptionsCategory::HIDDEN and then again in a real category
(or vice versa).
This is intentional behavior introduced in #13441: options unavailable
in a given binary (e.g. GUI args in bitcoind) are pre-registered as
hidden so shared bitcoin.conf files don't fail. In bitcoin-qt,
SetupServerArgs registers GUI args as hidden, then SetupUIArgs registers
them properly under OptionsCategory::GUI, triggering the assertion and
crashing on startup.
The fix relaxes the assertion to exclude HIDDEN from the cross-category
duplicate check, preserving the original intent of #13441 while still
catching unintentional duplicates between real categories.
In WriteSettings(), verify that writing to the stream and closing it
succeeded before returning true. This prevents RenameOver() from replacing
a valid settings.json with a corrupted or zero-byte file when write limits
or a full disk are encountered.
Additionally, update the ReadSettings() parse failure message to mention
power loss, full disk, or storage error as possible causes.
Fixes#35373
744d495019 ci: updated docs to reflect removal of REPO_USE_WARP_RUNNERS flag (Max Edwards)
Pull request description:
Small fix to the CI documentation that was missed in #35441
REPO_USE_WARP_RUNNERS is no longer there so can't be used.
ACKs for top commit:
willcl-ark:
ACK 744d495019
Tree-SHA512: b30b54ecb84f929623b4485609e8d0c8fd8371ad959471c81df84ffdaa8ad774a10eeca616b2e9c50f17998ebd27f5da33d3d758e67dac885fe8edc6dddf69fa
2a36d6a561 lint: Require scripted-diff script to succeed (Hodlinator)
Pull request description:
Previous version of commit-script-check.sh would succeed as long as git diff succeeded.
Can be verified through adding a failing scripted diff commit such as:
```
git commit --allow-empty -m $'scripted-diff: foo\n\n-BEGIN VERIFY SCRIPT-\nadsasd\n-END VERIFY SCRIPT-\n'
```
...and running...
```
cargo run --manifest-path ./test/lint/test_runner/Cargo.toml -- --lint=scripted_diff
```
ACKs for top commit:
maflcko:
lgtm ACK 2a36d6a561
furszy:
ACK 2a36d6a561
hebasto:
ACK 2a36d6a561.
Tree-SHA512: 160397cc009b18ccb3bc66c85c1b89404a81c9a85acee534638f4752577165aead56d6cfb21d9fcb424484bc6710658cd0f1d2f483d8298e9e3a2f5243ef8e8b
Previous version of commit-script-check.sh would succeed as long as git diff succeeded.
Can be verified through adding a failing scripted diff commit such as:
git commit --allow-empty -m $'scripted-diff: foo\n\n-BEGIN VERIFY SCRIPT-\nadsasd\n-END VERIFY SCRIPT-\n'
...and running...
cargo run --manifest-path ./test/lint/test_runner/Cargo.toml -- --lint=scripted_diff
32df86f1d8 argsman: Prevent duplicate option registration across categories (Pablo Martin)
Pull request description:
Follow-up to #28802
This PR enforces the invariant that option names are unique across categories.
-<ins>_**Rationale**_</ins>:
- While adapting the `argsman_tests.cpp` cases introduced in #28802 for the GNU-style parsing changes proposed in #33540, I noticed that the same option name can currently be registered in multiple categories.
- At present in `master`, this ambiguity is largely masked by the existing command-line parsing behaviour. However, it relies on assumptions about how options are interpreted based on their position. Future changes to option parsing, such as the GNU-style parsing proposed in #33540, may expose this ambiguity and lead to unexpected option resolution.
- To avoid ambiguous option resolution and make the distinction between global and command-specific options explicit, this PR adds validation in `AddArg()` preventing the same option name from being registered across different categories.
ACKs for top commit:
sedited:
ACK 32df86f1d8
ryanofsky:
Code review ACK 32df86f1d8. Thanks for the simplifications!
Tree-SHA512: 5d3cb951bd90c46cbd8205a229d49336c3a29e5100b7c50c5cba66979dd3cbd480ed42593caa272c6205eb7ac503af631a7e3531af0b726334e3448e17d5c0d5
55a4c946f6 test: descriptor: bare multisig at TOP level with 3 pubkeys is allowed (brunoerg)
Pull request description:
When running mutation testing for `src/script/descriptor.cpp`, I noticed that the following mutant was not killed:
```diff
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 3b73a40cce..29775ba75f 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -2393,7 +2393,7 @@ std::vector<std::unique_ptr<DescriptorImpl>> ParseScript(uint32_t& key_exp_index
return {};
}
if (ctx == ParseScriptContext::TOP) {
- if (providers.size() > 3) {
+ if (providers.size() >= 3) {
error = strprintf("Cannot have %u pubkeys in bare multisig; only at most 3 pubkeys", providers.size());
return {};
}
```
We current have a test case that verifies we cannot have 4 pubkeys in bare multisig, but we are not testing that exactly 3 pubkeys are allowed. This PR adds a test case for it.
ACKs for top commit:
darosior:
utACK 55a4c946f6
sedited:
ACK 55a4c946f6
Tree-SHA512: 2e0919b1cf0d991477a545d31de79c60f1378b027d88fb35e03cd3150d6bc9e3db8f231ccaee8f2800b464f50495546848e0f04e0152239296f1b5f6f38bb49a
2cf2b22ff1 depends: Drop trailing slash from `CMAKE_INSTALL_LIBDIR` (Hennadii Stepanov)
Pull request description:
1. The trailing slash is [redundant](https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html).
2. It causes [CMP0177](https://cmake.org/cmake/help/latest/policy/CMP0177.html)-specifc warnings when using CMake >=3.31.
On the master branch:
```
$ gmake -C depends freetype_configured
gmake: Entering directory '/home/hebasto/dev/bitcoin/depends'
Extracting freetype...
/home/hebasto/dev/bitcoin/depends/sources/freetype-2.11.1.tar.gz: OK
Preprocessing freetype...
patching file CMakeLists.txt
Configuring freetype...
-- The C compiler identification is GNU 15.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/gcc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Looking for unistd.h
-- Looking for unistd.h - found
-- Looking for fcntl.h
-- Looking for fcntl.h - found
CMake Warning (dev) at CMakeLists.txt:577 (install):
Policy CMP0177 is not set: install() DESTINATION paths are normalized. Run
"cmake --help-policy CMP0177" for policy details. Use the cmake_policy
command to set the policy and suppress this warning.
This warning is for project developers. Use -Wno-dev to suppress it.
CMake Warning (dev) at CMakeLists.txt:596 (install):
Policy CMP0177 is not set: install() DESTINATION paths are normalized. Run
"cmake --help-policy CMP0177" for policy details. Use the cmake_policy
command to set the policy and suppress this warning.
This warning is for project developers. Use -Wno-dev to suppress it.
CMake Warning (dev) at CMakeLists.txt:601 (install):
Policy CMP0177 is not set: install() DESTINATION paths are normalized. Run
"cmake --help-policy CMP0177" for policy details. Use the cmake_policy
command to set the policy and suppress this warning.
This warning is for project developers. Use -Wno-dev to suppress it.
-- Configuring done (0.2s)
-- Generating done (0.0s)
CMake Warning:
Manually-specified variables were not used by the project:
CMAKE_EXPORT_NO_PACKAGE_REGISTRY
-- Build files have been written to: /home/hebasto/dev/bitcoin/depends/work/build/x86_64-pc-linux-gnu/freetype/2.11.1-0539b8d2229/build
gmake: Leaving directory '/home/hebasto/dev/bitcoin/depends'
```
With this PR:
```
$ gmake -C depends freetype_configured
gmake: Entering directory '/home/hebasto/dev/bitcoin/depends'
Extracting freetype...
/home/hebasto/dev/bitcoin/depends/sources/freetype-2.11.1.tar.gz: OK
Preprocessing freetype...
patching file CMakeLists.txt
Configuring freetype...
-- The C compiler identification is GNU 15.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/gcc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Looking for unistd.h
-- Looking for unistd.h - found
-- Looking for fcntl.h
-- Looking for fcntl.h - found
-- Configuring done (0.2s)
-- Generating done (0.0s)
CMake Warning:
Manually-specified variables were not used by the project:
CMAKE_EXPORT_NO_PACKAGE_REGISTRY
-- Build files have been written to: /home/hebasto/dev/bitcoin/depends/work/build/x86_64-pc-linux-gnu/freetype/2.11.1-c25288632f9/build
gmake: Leaving directory '/home/hebasto/dev/bitcoin/depends'
```
ACKs for top commit:
ryanofsky:
Code review ACK 2cf2b22ff1. Seems good to clean up this warning, and change is a simplification
Tree-SHA512: 95f1d2fa01ebb62a0841948d2242d6baca6b72b2293b43c5d809d113328046cf0f44017db276408d5cf906ed75746333570dcef679fa72c5e38e5ade6af281e6
fae482b4e6 ci: Use GCC consistently in i686 task (MarcoFalke)
Pull request description:
According to the comment removed in commit fae0295a79, clang was only used to avoid OOM. Using GCC today should be fine.
(Meta note: The task can maybe even be removed in a few years, see https://github.com/bitcoin/bitcoin/pull/35230#issue-4397029749)
If OOM is an issue, or even if there is no issue, maybe `DEBUG=1` should be removed, because the `ci/test/00_setup_env_native_alpine_musl.sh` config is already checking `DEBUG=1` with GCC.
ACKs for top commit:
fanquake:
ACK fae482b4e6
sedited:
ACK fae482b4e6
Tree-SHA512: 6c8567fcb47a2c65d41ca277bb201d5bed462c5cd1c57048be995279e62f900c10f649304b3c8bebab352acf2bceef3122b8b699e9fc9845fe4c90a4bfaf0afe
Added a validation in AddArg() preventing the same option name from
being registered across different categories, avoiding ambiguous option
resolution and make the distinction between global and command-specific
options explicit.
fa98d44951 ci: Rewrite broken wrap-valgrind.sh to .py (MarcoFalke)
faf7e38973 ci: refactor: Avoid warning: INSTALL_BCC_TRACING_TOOLS: unbound variable (MarcoFalke)
Pull request description:
The first commit fixes an error about `INSTALL_BCC_TRACING_TOOLS` being unbound.
The second commit rewrites the wrap-valgrind Bash script to Python to fix the shellcheck SC2044 violation.
Without this, the script would fail in CI when a path with spaces was used:
```
...
+ /ci_container_base/ci/test/wrap-valgrind.sh
Wrap /ci_container_base/ci/scratch_ ...
mv: cannot stat '/ci_container_base/ci/scratch_': No such file or directory
Wrap ₿🧪_/out/bin/bitcoin ...
mv: cannot stat '₿🧪_/out/bin/bitcoin': No such file or directory
/ci_container_base/ci/test/wrap-valgrind.sh: line 14: ₿🧪_/out/bin/bitcoin: No such file or directory
/ci_container_base/ci/test/wrap-valgrind.sh: line 15: ₿🧪_/out/bin/bitcoin: No such file or directory
chmod: cannot access '₿🧪_/out/bin/bitcoin': No such file or directory
ACKs for top commit:
fanquake:
ACK fa98d44951 - tested both on x86_64
hebasto:
ACK fa98d44951, tested on Ubuntu 26.04. I also verified the actual content of the created wrappers.
Tree-SHA512: fd9ccdd08a3af3aa9431eef29e17da0d785c7fe57fd0bfc9a6afdd979dc8860f4f9f0153ba3bb4b516cdec7ef0e071b846fddc1f2d28395cbe9356d2e9c55fb7
d3e40af259 index: shorten indexer thread names (Lőrinc)
d69c46292d util: zero-pad thread number suffixes (Lőrinc)
41e531c4ab util: shorten `ThreadPool` worker names (Lőrinc)
Pull request description:
**Problem:** Linux limits thread names set through [`PR_SET_NAME`](https://man7.org/linux/man-pages/man2/PR_SET_NAME.2const.html) to 15 visible bytes:
> The name can be up to 16 bytes long, including the terminating null byte.
Bitcoin Core prefixes system thread names with `b-`, leaving only 13 bytes for the thread-specific part.
This truncates longer indexer names in system tools, for example `b-coinstatsindex` and `b-txospenderindex`.
It also makes verbose worker suffixes like `b-http_pool_N` spend much of the available space; the current HTTP worker names fit, but the generic suffix leaves less room for longer pool names.
The same limit is documented in the existing thread-name helper:
8b49e2dd4e/src/util/threadnames.cpp (L25)
This was noticed during review of https://github.com/bitcoin/bitcoin/pull/31132#discussion_r3146688138
**Fix:** Shorten the OS-visible thread names while keeping public index identifiers unchanged.
`ThreadPool` workers now use a zero-padded dotted numeric suffix, so HTTP workers are named like `b-http.xx`.
`CCheckQueue` worker suffixes are zero-padded as well, so script-check workers are named like `b-scriptch.xx`.
Indexer sync threads now pass display and thread names separately at each `BaseIndex` call site.
The current indexer thread names use compact `idx`-suffixed names that fit within the Linux limit after the `b-` prefix:
```text
txindex -> txidx
basic block filter index -> blkfltbscidx
coinstatsindex -> coinstatsidx
txospenderindex -> txospenderidx
```
Indexer display names, `getindexinfo` keys, command-line options, and on-disk index paths are unchanged.
**Testing:** See https://godbolt.org/z/oWonrTKcj for a simple reproducer.
Alternatively, start `bitcoind` on Linux with the affected indexes enabled and read the kernel-visible thread names.
Before this change, the relevant names were truncated or used longer forms:
```text
b-txindex
b-basic block f
b-coinstatsinde
b-http_pool_15
b-txospenderind
```
After this change, the same check shows compact, untruncated names:
```text
b-txidx
b-blkfltbscidx
b-coinstatsidx
b-http.15
b-txospenderidx
```
ACKs for top commit:
maflcko:
re-ACK d3e40af259👳
sedited:
ACK d3e40af259
winterrdog:
re-ACK d3e40af259
hodlinator:
re-ACK d3e40af259
Tree-SHA512: 0b4c087661eb81e767fb2c2a1ce2dd54e6593888a7d30402e76c845a84dff5550e3ad72fee39b136f7f5214f051647c0c1f284e3265f8a614d1028f7b49d76da
`BaseIndex` currently uses the same name for logs, `getindexinfo`, prune locks, and the sync thread.
Linux truncates system thread names to 15 visible bytes after the `b-` prefix, so long indexer names are clipped in system tools.
Pass a separate thread name explicitly at each `BaseIndex` call site and shorten the OS-visible indexer names to `txidx`, `blkfltbscidx`, `coinstatsidx`, and `txospenderidx`.
Add an `Assume` to `ThreadRename()` so future OS-visible thread names keep fitting the same Linux limit.
The public index names, `getindexinfo` keys, command-line options, and on-disk paths stay unchanged.
Co-authored-by: winterrdog <winterrdog@users.noreply.github.com>
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: sedited <seb.kung@gmail.com>
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Thread names with numeric suffixes are easier to scan when the suffixes use a fixed width.
Format `ThreadPool` and script-check worker suffixes as two digits while keeping the compact dotted convention.
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
ThreadPool workers currently format their names as `name_pool_N`.
Linux truncates the system thread name to 15 visible bytes, so `b-http_pool_N` spends much of that space on the suffix.
Use a dotted numeric suffix so HTTP worker names become `b-http.N`, leaving more room for longer pool names.
58cc2a0453 test: make TestChain100Setup's m_clock timestamp more readable (Hao Xu)
Pull request description:
The timestamp 1598887952 is used as the initial value of the node's mocked wall-clock in TestChain100Setup. Comment its UTC date to make the magic number more readable.
ACKs for top commit:
maflcko:
lgtm ACK 58cc2a0453
Tree-SHA512: bd26977dcbdc6e2b448152dd0c2c71cee4ed73eb64ea32be3cac1e0a3f432f588b9f3261037be804d308e713163c2b47350c020327e21d7e9a9fe9a2827cdf45
726e196ef2 ci: inline runner selection (will)
Pull request description:
We are currently sometimes backlogged on waiting for runner selection.
Selecting Warp or GitHub-hosted runners directly from the repository context avoids serializing all CI jobs behind a metadata job. This keeps forks on public runners while allowing upstream jobs to schedule immediately on the intended runner labels.
This will make the changes needed for "a fork also wanting to use warp runners" slightly larger, but I think that number of people is zero.
ACKs for top commit:
m3dwards:
ACK 726e196ef2
Tree-SHA512: 14f3f22fbc53b3f65fce48bd08aef8db8d60a37409fd8b4b5b128073cf81408f7d3135c1e78fe3f93af8c88b3ee8c1b770e445b1ee7d5650ea91b329bc7ff777
The timestamp 1598887952 is used as the initial value of the node's
mocked wall-clock in TestChain100Setup. Comment its UTC date to make
the magic number more readable.
059edf1908 guix: Fix "Ignoring empty string" CMake warning for non-Linux hosts (Hennadii Stepanov)
2d86083fd4 guix: Drop redundant CMake `--verbose` options (Hennadii Stepanov)
Pull request description:
This PR introduces the following CMake-related improvements to [`contrib/guix/libexec/build.sh`](https://github.com/bitcoin/bitcoin/blob/master/contrib/guix/libexec/build.sh):
1. Drop redundant CMake `--verbose` options, as verbose output is already controlled by the [`VERBOSE`](https://cmake.org/cmake/help/latest/envvar/VERBOSE.html) environment variable, which has been exported since 85f4a4b082.
2. Fixes the "Ignoring empty string" CMake warning for non-Linux hosts.
ACKs for top commit:
fanquake:
ACK 059edf1908
Tree-SHA512: 8f32c29815e1417cba9cd61fe07704f01788c800cd77f571f0693de3d5d63cd8e0b6f6a77dbf68f5de90e5572385571ba82a1c24caac955b7a7617fe12399d13
d92a20b310 iwyu: Fix warning in `bench/pool.cpp` (Hennadii Stepanov)
Pull request description:
This PR resolves a silent merge conflict between recently merged #35414 and #35101 (47d68cd981).
ACKs for top commit:
fanquake:
ACK d92a20b310
Tree-SHA512: e496c56e3fba8c0030f4cfc79f5f678ac07aed87fb24738ed074ba171bfbcf4ea732f542b07dcd12247af5e6799d80f294bdc05ba8216a9465cf5192380bf818
Using shell parameter expansion ensures that on non-Linux hosts the
build script won't pass an empty string argument to CMake. This resolves
the "Ignoring empty string" warning.
6751a323c0 iwyu: Fix warnings in `src/bench` and treat them as error (Hennadii Stepanov)
a6ed29d6c2 bench, refactor: Use `std::string_view` for `BenchRunner` ctor parameter (Hennadii Stepanov)
Pull request description:
This PR addresses [this](https://github.com/bitcoin/bitcoin/pull/35011#discussion_r3323359707) comment:
> I had the impression I already fixed bench in https://github.com/bitcoin/bitcoin/pull/30716 two years ago, but I guess it isn't yet enforced.
>
> Could do that as a next step?
The first two commits act as prerequisites. See the commit messages for details.
The third commit additionally ensures that our drop-in header replacements are used instead of system headers:
- `util/check.h`:10dfdd4b9f/src/util/check.h (L11-L13)
- `util/time.h`:10dfdd4b9f/src/util/time.h (L9-L10)
ACKs for top commit:
maflcko:
re-ACK 6751a323c0📃
BrandonOdiwuor:
ACK 6751a323c0
Tree-SHA512: 159ee734a83dcba3c914682be4b119549e1e4269a43d34c52903e76056d537a2ae02c2f5f4e3adff1b4230082b8ed267c04164abc83b16f717b34fba6e03e359
9fae7e9886 test: doc: remove `--perf` profiling from functional test framework (Sebastian Falbesoner)
Pull request description:
This PR is an alternative to #35509. Rather than fixing, it removes the `--perf` option / `profile_with_perf` context manager features (both introduced in #14519) from the functional test framework. Given that no developer apparently even noticed that it's been broken for more than a year, we can conclude that it's largely unused and getting rid of it to reduce maintenance burden seems a reasonable choice. So far, neither in #35509 nor [on IRC](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2026-06-12#1227556;) anyone has signaled strong interest to use or wanting to keep this feature.
The corresponding test documentation is removed as well, though the mentioning of `perf` profiling is still kept in the general developer docs -- only the reference to functional test framework integration is removed there.
ACKs for top commit:
l0rinc:
lightweight code review ACK 9fae7e9886
maflcko:
lgtm ACK 9fae7e9886 While this seems useful, no one using it is a good reason to remove it. If a user comes after this is merged, it should be trivial to revert.
Tree-SHA512: cf7bdce72aed877c7dfa52a230840e0729e74b00f0c40c6ceecf04957707f4699f123c2cde5fc1c4ee21f4b1b14319c74acc78cd8066eab2b9a4d6efb4d11539
9bfdde74b5 guix: add package.sh (fanquake)
Pull request description:
Split out packaging code, so that it can be re-used by build scripts. This is the second (mostly move-only) commit split out from #25573, before the changes that begin modifying the build.
ACKs for top commit:
hebasto:
ACK 9bfdde74b5.
willcl-ark:
ACK 9bfdde74b5
Tree-SHA512: da5a8b0f12054e3af100810a7963eb62f8db54c5f003a4e63405fd69b4118387d6392ebfa99468de70457cbaf6575b2733eb9f8d8114ec9dd625d7f16c066ba5
406c2348dd rpc: tighten setmocktime upper bound to UINT32_MAX (stringintech)
Pull request description:
The previous upper bound for `setmocktime` was `std::chrono::nanoseconds::max()` converted to seconds (~year 2262). This was too permissive in two ways:
1. Paths that add an offset to the mocked time can overflow `int64_t` (caught by UBSan). For example, `ContextualCheckBlockHeader` adds a constant to the current time for its future-time check. (see [comment](https://github.com/bitcoin/bitcoin/pull/35496#issuecomment-4678552371))
2. Paths that assign the mocked time to a `uint32_t` field silently truncate it (caught by the integer sanitizer). For example, `miner.cpp` assigns `NodeClock::now()` directly to `pblock->nTime`. (see [comment](https://github.com/bitcoin/bitcoin/pull/35496#issuecomment-4679331674))
`UINT32_MAX` is the natural ceiling since block header `nTime` is `uint32_t`, making mocked values beyond it meaningless for anything consensus-related.
ACKs for top commit:
sedited:
ACK 406c2348dd
winterrdog:
ACK 406c2348dd
Tree-SHA512: 4dc5f5125ed48a11a62661446870dbd2b3b29c30b04094c3f2b4293a2a73ed61ce785e15666b3549da7c4a08055a5d27b0a5598061a432eeda0e69495c37b426
