`SocketHandlerConnected()` adapted from CConnman
Testing this requires adding a new feature to the SocketTestingSetup,
inserting a "request" payload into the mock client that connects
to us.
This commit also moves IOErrorIsPermanent() from sock.cpp to sock.h
so it can be called from the socket handler in httpserver.cpp
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Socket handling methods are copied from CConnMan:
`CConnman::GenerateWaitSockets()`
`CConnman::SocketHandlerListening()`
`CConnman::ThreadSocketHandler()` and `CConnman::SocketHandler()` are combined into ThreadSocketHandler()`.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Introduce a new low-level socket managing class `HTTPServer`.
BindAndStartListening() was copied from CConnMan's BindListenPort()
in net.cpp and modernized.
Unit-test it with a new class `SocketTestingSetup` which mocks
`CreateSock()` and will enable mock client I/O in future commits.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
This commit is a no-op to isolate HTTP methods and objects that
depend on libevent. Following commits will add replacement objects
and methods in a new namespace for testing and review before
switching over the server.
The utility can not be opinionated about CR or SP on either end of a
line it reads. That decision is up to the caller and in fact in the
case of HTTP should be allowed in some places and rejected in others.
Replace TrimStringView() with more surgical operation.
394e473d42 coins: compact chainstate in background (Lőrinc)
aa021b26f3 validation: randomly compact chainstate (Lőrinc)
b10889d107 coins: test chainstate flush baseline (Lőrinc)
Pull request description:
**Problem:** https://github.com/bitcoin-core/leveldb-subtree/pull/61 disabled read-triggered seek compactions to avoid large chainstate write amplification from random UTXO lookups.
That avoids repeated read-driven rewrites, but it also removes opportunistic cleanup that previously helped compact old chainstate data.
After IBD, normal chainstate churn can leave obsolete entries behind until ordinary LevelDB compaction naturally reaches the affected levels, keeping the chainstate database larger than necessary.
Also, chainstates created by pre-29 nodes can contain thousands of files from the old 2 MiB LevelDB table target.
After the mmap limit dropped back to 1000 and seek compaction was disabled, continuing from such a chainstate can leave many table reads on the non-mmap path until the database is compacted.
**Fix:** After each completed post-IBD full chainstate flush, give the chainstate a 1/320 chance to compact.
With roughly hourly full flushes, this averages about once every two weeks and makes a six-month stretch without compaction about a one-in-a-million event.
The randomized recurring trigger spreads compactions across nodes and keeps maintenance stateless, without storing last-compaction height or timestamp metadata in the chainstate database.
Compaction runs on a background thread (`utxocompact`) so validation only schedules the work.
Partially fixes#35298 and #35457
ACKs for top commit:
sipa:
ACK 394e473d42
optout21:
ACK 394e473d42
andrewtoth:
ACK 394e473d42
sedited:
ACK 394e473d42
Tree-SHA512: 536a298f86ca7d87a2dd5428354b05b693931c090c7a4afd4d09b95ce53e2bd8319e01c940489a1520d8cb1010a7ea9724d2c18d2e12887bd5ec70cb68c03517
855a3fee88 scripted-diff: Rename SteadyClockContext to FakeSteadyClock (Hao Xu)
Pull request description:
See: https://github.com/bitcoin/bitcoin/pull/35536#issuecomment-4715945494
`SteadyClockContext` and `FakeNodeClock` are both LimitOne RAII helpers that mock a clock in tests -- the steady clock and the node clock, respectively. Rename the former so the two follow a consistent FakeXClock naming scheme.
ACKs for top commit:
maflcko:
lgtm ACK 855a3fee88
w0xlt:
ACK 855a3fee88
sedited:
ACK 855a3fee88
Tree-SHA512: 72fd8dd3217d19f4b9f587e379a0aea2b7760ca6d61be6c363e7c06757cba40487df8b40cf3d62f9f9256fb4d0373610dd6f62c6e9ef3edde02265fc967a6bdf
fab2874269 lint: Require scripted-diff script to succeed (MarcoFalke)
Pull request description:
Currently, scripted diffs may silently pass with errors.
Fix this issue by calling the script from a Bash instance with error checking enabled: `bash -o errexit -o nounset -o pipefail -c "$SCRIPT"`.
Also, use Bash (not sh) when launching the script itself, because Bash is required anyway.
Can be tested by running something like this and observing the behavior before and after:
```
git commit --allow-empty -m $'scripted-diff: foo\n\n-BEGIN VERIFY SCRIPT-\n false;falseasfsafsaf;true;false|cat; echo "${NO_UN_SET}"|cat \n-END VERIFY SCRIPT-\n' && ./test/lint/commit-script-check.sh HEAD~..HEAD ; echo $?
```
Alternatively, an ancient brittle script can be tested:
```
./test/lint/commit-script-check.sh fb65dde147f63422c4148b089c2f5be0bf5ba80f~..fb65dde147f63422c4148b089c2f5be0bf5ba80f
ACKs for top commit:
hodlinator:
ACK fab2874269
sedited:
ACK fab2874269
Tree-SHA512: e3e8167e150be45a096d4883057640eb5624456f21b134cfa901fe490e5afb192855e55a752cb6121399314f65db155544a60645a101c39a084a62de4af23298
8f0354995b depends: latest config.guess & config.sub (fanquake)
Pull request description:
Pull the latest versions from upstream.
ACKs for top commit:
hebasto:
ACK 8f0354995b, verified using the upstream [repo](https://cgit.git.savannah.gnu.org/cgit/config.git).
Tree-SHA512: 6ea7ce70a7db614d32f0aea47006892766f6abb41e31fef25cadd4423a0f6e42f8500f03ea70a3990c2628ad1b534b7c52c19914289375f87289ca34349075b2
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
SteadyClockContext and FakeNodeClock are both LimitOne RAII helpers that mock a
clock in tests -- the steady clock and the node clock, respectively. Rename the
former so the two follow a consistent FakeXClock naming scheme.
-BEGIN VERIFY SCRIPT-
sed -i 's/SteadyClockContext/FakeSteadyClock/g' $(git grep -l SteadyClockContext)
-END VERIFY SCRIPT-
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.
