When dumptxoutset rollback is used and competing forks exist at the target
height, the InvalidateBlock call would cause a reorg to the competing fork
instead of rolling back to the desired block on the main chain.
Fix by invalidating all competing fork blocks first before invalidating
the main chain block. This prevents reorgs during rollback and ensures
the snapshot is created from the correct main chain block.
The fix enhances the TemporaryRollback class to:
- Identify competing fork blocks at heights above the target
- Invalidate all fork blocks before main chain invalidation
- Restore all invalidated blocks in the destructor
Add test coverage for dumptxoutset rollback behavior when competing
forks exist at the target rollback height. The test verifies that
the current implementation fails in this scenario, establishing
a baseline for future fixes.
b81f37031c p2p: Increase tx relay rate (Anthony Towns)
Pull request description:
In the presence of smaller transactions on the network, blocks can sustain a higher relay rate than 7tx/second. In this event, the per-peer inventory queues can grow too large.
This commit bumps the rate up to 14 tx/s (for inbound peers), increasing the safety margin by a factor of 2.
Outbound peers continue to receive relayed transactions at 2.5x the rate of inbound peers, for a rate of 35tx/second.
ACKs for top commit:
sipa:
ACK b81f37031c
achow101:
ACK b81f37031c
darosior:
utACK b81f37031c.
glozow:
utACK b81f37031c
Tree-SHA512: 854ea0824d5f4c629f1dceb9ee61cc9226c8f0d4d26664737e68db917f65341d4800362ab55ed32673db920b2b59aa116b4cb9ee063367b2e43c94a904b41c08
67f632b6de net: remove unnecessary casts in socket operations (Matthew Zipkin)
Pull request description:
During review of https://github.com/bitcoin/bitcoin/pull/32747 several casting operations were questioned in existing code that had been copied or moved. That lead me to find a few other similar casts in the codebase.
It turns out that since the `Sock` class wraps syscalls with its own internal casting (see https://github.com/bitcoin/bitcoin/pull/24357 and https://github.com/bitcoin/bitcoin/pull/20788 written in 2020-2022) we no longer need to cast the arguments when calling these functions. The original argument-casts are old and were cleaned up a bit in https://github.com/bitcoin/bitcoin/pull/12855 written in 2018.
The casting is only needed for windows compatibility, where those syscalls require a data argument to be of type `char*` specifically:
https://learn.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-getsockopt
```
int getsockopt(
[in] SOCKET s,
[in] int level,
[in] int optname,
[out] char *optval,
[in, out] int *optlen
);
```
but on POSIX the argument is `void*`:
https://www.man7.org/linux/man-pages/man2/getsockopt.2.html
```
int getsockopt(socklen *restrict optlen;
int sockfd, int level, int optname,
void optval[_Nullable restrict *optlen],
socklen_t *restrict optlen);
```
ACKs for top commit:
Raimo33:
ACK 67f632b6de
achow101:
ACK 67f632b6de
hodlinator:
ACK 67f632b6de
vasild:
ACK 67f632b6de
davidgumberg:
ACK 67f632b6de
Tree-SHA512: c326d7242698b8d4d019f630fb6281398da2773c4e5aad1e3bba093a012c2119ad8815f42bd009e61a9a90db9b8e6ed5c75174aac059c9df83dd3aa5618a9ba6
168360f4ae coins: warn on oversized -dbcache (Lőrinc)
6c720459be system: add helper for fetching total system memory (Lőrinc)
Pull request description:
### Summary
Oversized allocations can cause out-of-memory errors or [heavy swapping](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663637321), [grinding the system to a halt](https://x.com/murchandamus/status/1964432335849607224).
### Fix
Added a minimal system helper to query total physical RAM on [Linux/macOS/Windows](https://stackoverflow.com/a/2513561) (on unsupported platforms we just disable this warning completely).
The added test checks if the value is roughly correct by checking if the CI platforms are returning any value and if the value is at least 1 GB (as a simple property test checking if the unit size is correct, e.g. doesn't return megabytes or bits).
### Details
`LogOversizedDbCache()` now emits a startup warning if the configured `-dbcache` exceeds a cap derived from system RAM, using the same parsing/clamping as cache sizing via `CalculateDbCacheBytes()`. This isn't meant as a recommended setting, rather a likely upper limit.
Note that we're not modifying the set value, just issuing a warning.
Also note that the 75% calculation is rounded for the last two numbers since we have to divide first before multiplying, otherwise we wouldn't stay inside `size_t` on 32-bit systems - and this was simpler than casting back and forth.
We could have chosen the remaining free memory for the warning (e.g. warn if free memory is less than 1 GiB), but this is just a heuristic, we assumed that on systems with a lot of memory, other processes are also running, while memory constrained ones run only Core.
### Cap
If total RAM < 2 GiB, cap is `DEFAULT_DB_CACHE` (`450 MiB`), otherwise it's 75% of total RAM.
The threshold is chosen to be close to values commonly used in [raspiblitz](https://github.com/raspiblitz/raspiblitz/blob/dev/home.admin/_provision.setup.sh#L98-L115) for common setups:
| Total RAM | `dbcache` (MiB) | raspiblitz % | proposed cap (MiB) |
|----------:|----------------:|-------------:|-------------------:|
| 1 GiB | 512 | 50.0% | 450* |
| 2 GiB | 1536 | 75.0% | 1536 |
| 4 GiB | 2560 | 62.5% | 3072 |
| 8 GiB | 4096 | 50.0% | 6144 |
| 16 GiB | 4096 | 25.0% | 12288 |
| 32 GiB | 4096 | 12.5% | 24576 |
[Umbrel issues](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663816367) also mention 75% being the upper limit.
### Reproducer
Starting `bitcoind` on an 8 GiB rpi4b with a dbcache of 7 GiB:
> ./build/bin/bitcoind -dbcache=7000
warns now as follows:
```
2025-09-07T17:24:29Z [warning] A 7000 MiB dbcache may be too large for a system memory of only 7800 MiB.
Warning: A 7000 MiB dbcache may be too large for a system memory of only 7800 MiB.
2025-09-07T17:24:29Z Cache configuration:
2025-09-07T17:24:29Z * Using 2.0 MiB for block index database
2025-09-07T17:24:29Z * Using 8.0 MiB for chain state database
2025-09-07T17:24:29Z * Using 6990.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
```
### Manual testing
Besides the [godbolt](https://godbolt.org/z/ec81Tjvrj) reproducers for the new total memory method, we also tested the warnings manually on:
- [x] Apple M4 Max, macOS 15.6.1
- [x] Intel Core i9-9900K, Ubuntu 24.04.2 LTS
- [x] Raspberry Pi 4 Model B, Armbian Linux 6.12.22-current-bcm2711
- [x] Intel Xeon x64, Windows 11 Home Version 24H2, OS Build 26100.4351
ACKs for top commit:
achow101:
ACK 168360f4ae
w0xlt:
reACK 168360f4ae
hodlinator:
re-ACK 168360f4ae
danielabrozzoni:
reACK 168360f4ae
Tree-SHA512: aa0c9b1034d55a6a4212685a19715d8cd89668ab7c33c688711a15559e6ad81aa65f3cd8b488c91385306e1e16cd9eeefa8f659ba90ef19ce9c7a2e64f8b561a
e9c52272eb test: Avoid interface_ipc.py Duplicate ID errors (Ryan Ofsky)
Pull request description:
This change should fix issue https://github.com/bitcoin/bitcoin/issues/33417 reported by zaidmstrr. It's possible to reproduce the `mp/proxy.capnp:0: failed: Duplicate ID @0xcc316e3f71a040fb` error by installing libmultiprocess system-wide, or to one of the locations listed in the python test's `imports` list before the local libmultiprocess subtree, and then running the test.
ACKs for top commit:
zaidmstrr:
Tested ACK [e9c5227](e9c52272eb)
Tree-SHA512: 5df7fe767989b91245ce96f7c43b6767b7af49ec6c7007175e462341ffd69e161f21632697804060ce286b3e102a8d141a57a53f7e0e32299ef9a3a69ca8794a
Oversized allocations can cause out-of-memory errors or [heavy swapping](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663637321), [grinding the system to a halt](https://x.com/murchandamus/status/1964432335849607224).
`LogOversizedDbCache()` now emits a startup warning if the configured `-dbcache` exceeds a cap derived from system RAM, using the same parsing/clamping as cache sizing via CalculateDbCacheBytes(). This isn't meant as a recommended setting, rather a likely upper limit.
Note that we're not modifying the set value, just issuing a warning.
Also note that the 75% calculation is rounded for the last two numbers since we have to divide first before multiplying, otherwise we wouldn't stay inside size_t on 32-bit systems - and this was simpler than casting back and forth.
We could have chosen the remaining free memory for the warning (e.g. warn if free memory is less than 1 GiB), but this is just a heuristic, we assumed that on systems with a lot of memory, other processes are also running, while memory constrained ones run only Core.
If total RAM < 2 GiB, cap is `DEFAULT_DB_CACHE` (`450 MiB`), otherwise it's 75% of total RAM.
The threshold is chosen to be close to values commonly used in [raspiblitz](https://github.com/raspiblitz/raspiblitz/blob/dev/home.admin/_provision.setup.sh#L98-L115) for common setups:
| Total RAM | `dbcache` (MiB) | raspiblitz % | proposed cap (MiB) |
|----------:|----------------:|-------------:|-------------------:|
| 1 GiB | 512 | 50.0% | 450* |
| 2 GiB | 1536 | 75.0% | 1536 |
| 4 GiB | 2560 | 62.5% | 3072 |
| 8 GiB | 4096 | 50.0% | 6144 |
| 16 GiB | 4096 | 25.0% | 12288 |
| 32 GiB | 4096 | 12.5% | 24576 |
[Umbrel issues](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663816367) also mention 75% being the upper limit.
Starting `bitcoind` on an 8 GiB rpi4b with a dbcache of 7 GiB:
> ./build/bin/bitcoind -dbcache=7000
warns now as follows:
```
2025-09-07T17:24:29Z [warning] A 7000 MiB dbcache may be too large for a system memory of only 7800 MiB.
2025-09-07T17:24:29Z Cache configuration:
2025-09-07T17:24:29Z * Using 2.0 MiB for block index database
2025-09-07T17:24:29Z * Using 8.0 MiB for chain state database
2025-09-07T17:24:29Z * Using 6990.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
```
Besides the [godbolt](https://godbolt.org/z/EPsaE3xTj) reproducers for the new total memory method, we also tested the warnings manually on:
- [x] Apple M4 Max, macOS 15.6.1
- [x] Intel Core i9-9900K, Ubuntu 24.04.2 LTS
- [x] Raspberry Pi 4 Model B, Armbian Linux 6.12.22-current-bcm2711
- [x] Intel Xeon x64, Windows 11 Home Version 24H2, OS Build 26100.4351
Co-authored-by: stickies-v <stickies-v@protonmail.com>
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: w0xlt <woltx@protonmail.com>
Added a minimal system helper to query total physical RAM on [Linux/macOS/Windows](https://stackoverflow.com/a/2513561) (on other platforms we just return an empty optional).
The added test checks if the value is roughly correct by checking if the CI platforms are returning any value and if the value is at least 1 GiB and not more than 10 TiB.
The max value is only validated on 64 bits, since it's not unreasonable for 32 bits to have max memory, but on 64 bits it's likely an error.
https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/ns-sysinfoapi-memorystatusex
> ullTotalPhys The amount of actual physical memory, in bytes.
https://man7.org/linux/man-pages/man3/sysconf.3.html:
> _SC_PHYS_PAGES The number of pages of physical memory. Note that it is possible for the product of this value and the value of _SC_PAGESIZE to overflow.
> _SC_PAGESIZE Size of a page in bytes. Must not be less than 1.
See https://godbolt.org/z/ec81Tjvrj for further details
This change should fix issue https://github.com/bitcoin/bitcoin/issues/33417
reported by zaidmstrr. It's possible to reproduce the `mp/proxy.capnp:0:
failed: Duplicate ID @0xcc316e3f71a040fb` error by installing libmultiprocess
system-wide, or to one of the locations listed in the python test's `imports`
list before the local libmultiprocess subtree, and then running the test.
47d79db8a552 Merge bitcoin-core/libmultiprocess#201: bug: fix mptest hang, ProxyClient<Thread> deadlock in disconnect handler
f15ae9c9b9fb Merge bitcoin-core/libmultiprocess#211: Add .gitignore
4a269b21b8c8 bug: fix ProxyClient<Thread> deadlock if disconnected as IPC call is returning
85df96482c49 Use try_emplace in SetThread instead of threads.find
ca9b380ea91a Use std::optional in ConnThreads to allow shortening locks
9b0799113557 doc: describe ThreadContext struct and synchronization requirements
d60db601ed9b proxy-io.h: add Waiter::m_mutex thread safety annotations
4e365b019a9f ci: Use -Wthread-safety not -Wthread-safety-analysis
15d7bafbb001 Add .gitignore
fe1cd8c76131 Merge bitcoin-core/libmultiprocess#208: ci: Test minimum cmake version in olddeps job
b713a0b7bfbc Merge bitcoin-core/libmultiprocess#207: ci: output CMake version in CI script
0f580397c913 ci: Test minimum cmake version in olddeps job
d603dcc0eef0 ci: output CMake version in CI script
git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 47d79db8a5528097b408e18f7b0bae11a6702d26
7584a4fda9 cmake: Install `bitcoin` manpage (Hennadii Stepanov)
Pull request description:
This PR is an amendment to https://github.com/bitcoin/bitcoin/pull/31375.
ACKs for top commit:
ryanofsky:
Code review ACK 7584a4fda9.
Tree-SHA512: 66810c1d65fa8ae469b8161a5f807aa7b43a7b18e88d40b05617c7110b2e03e07bcb8f310c1736fb2c3738e274fc524032ff5d34d5c644824a4edd64372f1e9f
f563ce9081 net: Do not apply whitelist permission to onion inbounds (Martin Zumsande)
Pull request description:
Tor inbound connections do not reveal the peer's actual network address. Do not apply whitelist permissions to them since address-based matching is ineffective.
ACKs for top commit:
darosior:
ACK f563ce9081
furszy:
ACK f563ce9081
vasild:
ACK f563ce9081
Tree-SHA512: 49ae70e382fc2f78b7073553fe649a6843a41214b2986ea7f77e285d02b7bd00fe0320a1b71d1aaca08713808fb14af058f0b1f19f19adb3a77b97cb9d3449ce
Tor inbound connections do not reveal the peer's actual network address.
Therefore do not apply whitelist permissions to them.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
0a26731c4c test: Add submitblock test in interface_ipc (TheCharlatan)
Pull request description:
Expands the ipc mining test a bit with submitting a solved block and checking its validity.
ACKs for top commit:
Sjors:
ACK 0a26731c4c
marcofleon:
code review ACK 0a26731c4c
zaidmstrr:
Tested ACK [0a26731](0a26731c4c)
Tree-SHA512: 35c87d88496eec469bddedf2ae82c494626abb47ae15d5a45d6ab0400199c86501199c3e569e83836549830042be76b197b470e1100a317bdfef2578a9d5a92f
These methods in the Sock class wrap corresponding syscalls,
accepting void* arguments and casting to char* internally, which is
needed for Windows support and ignored on other platforms because
the syscall itself accepts void*:
Send()
Recv()
GetSockOpt()
SetSockOpt()
bdf01c6f61 test: Prevent disk space warning during node_init_tests (Ryan Ofsky)
Pull request description:
mzumsande pointed out https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3286964369 that this test was print a warning:
```
Warning: Disk space for "/tmp/test_common bitcoin/node_init_tests/init_test/bf78678cb7723a3e84b5/blocks" may not accommodate the block files. Approximately 810 GB of data will be stored in this directory.
```
Fix by setting regtest instead of mainnet network before running the test.
ACKs for top commit:
achow101:
ACK bdf01c6f61
Eunovo:
Tested ACK bdf01c6f61:
janb84:
ACK bdf01c6f61
l0rinc:
tested ACK bdf01c6f61
mzumsande:
utACK bdf01c6f61
enirox001:
utACK bdf01c6
Tree-SHA512: ac4e1e48246c84a4c4b80ccb25e962b0090359ab0e541ee4f1a9e18ac9da8ec35a78c9a55501d231423053e945ff785862f0db141d4b620d622327670c764f8c
mzumsande pointed out https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3286964369 that this test was causing a warning:
Warning: Disk space for "/tmp/test_common bitcoin/node_init_tests/init_test/bf78678cb7723a3e84b5/blocks" may not accommodate the block files. Approximately 810 GB of data will be stored in this directory.
Fix by setting regtest instead of mainnet network before running the test.
113a422822 wallet: Add m_cached_from_me to cache "from me" status (Ava Chow)
609d265ebc test: Add a test for anchor outputs in the wallet (Ava Chow)
c40dc822d7 wallet: Throw an error in sendall if the tx size cannot be calculated (Ava Chow)
39a7dbdd27 wallet: Determine IsFromMe by checking for TXOs of inputs (Ava Chow)
e76c2f7a41 test: Test wallet 'from me' status change (Ava Chow)
Pull request description:
One of the ways that the wallet would determine if a transaction was sent from the wallet was by checking if the total amount being spent by a transaction from outputs known to the wallet was greater than 0. This has worked fine until recently since there was no reason for 0-value outputs to be created. However, with ephemeral dust and P2A, it is possible to create standard 0-value outputs, and the wallet was not correctly identifying the spends of such outputs. This PR updates `IsFromMe` to only check whether the wallet knows any of the inputs, rather than checking the debit amount of a transaction.
Additionally, a new functional test is added to test for this case, as well as a few other anchor output related scenarios. This also revealed a bug in `sendall` which would cause an assertion error when trying to spend all of the outputs in a wallet that has anchor outputs.
Fixes#33265
ACKs for top commit:
rkrux:
lgtm ACK 113a422822
enirox001:
Tested ACK 113a422. Ran the full functional test suite including `wallet_anchor.py`; all tests passed. Fix for 0 value anchor detection and sendall size errors looks good. LGTM.
furszy:
ACK 113a422822
Tree-SHA512: df2ce4b258d1875ad0b4f27a5b9b4437137a5889a7d5ed7fbca65f904615e9572d232a8b8d070760f75ac168c1a49b7981f6b5052308575866dc610d191ca964
93a29ff283 trace: Workaround GCC bug compiling with old systemtap (Luke Dashjr)
Pull request description:
ACKs for top commit:
0xB10C:
lgtm ACK 93a29ff283 - I did not test this.
Tree-SHA512: 9ce9ed8b7733af721134462073a3417e52d67e9e9853eebbddfa795842b381de98e28756ebfa6652536cbfdd08181142eccd198f4dc00a57d8748801b362b4b7
b736052e39 ci: always use tag for LLVM checkout (fanquake)
Pull request description:
Rather than trying to match the apt installed clang version, which is prone to intermittent issues. i.e #33345.
ACKs for top commit:
davidgumberg:
ACK b736052e39
willcl-ark:
ACK b736052e39
Tree-SHA512: 8e3fcc8219f573cec65941576c7995f21cae3330bcdbf615f799e8c5facd1146d3239a7284e9af7b013c37170ddf7435d7df6d2966f63fe7b4a8e4937311ff36
fa96a4afea ci: Enable CI_LIMIT_STACK_SIZE=1 in i686_no_ipc task (MarcoFalke)
facfde2cdc test: Fix CLI_MAX_ARG_SIZE issues (MarcoFalke)
Pull request description:
`CLI_MAX_ARG_SIZE` has many edge case issues:
* It seems to be lower on some systems, but it is unknown how to reproduce locally: https://github.com/bitcoin/bitcoin/pull/33079#issuecomment-3139957274
* `MAX_ARG_STRLEN` is a limit per arg, but we probably want "The maximum length of [all of] the arguments": See https://www.man7.org/linux/man-pages/man3/sysconf.3.html, section `ARG_MAX - _SC_ARG_MAX`.
* It doesn't account for the additional args added by the `bitcoin` command later on: 73220fc0f9/src/bitcoin.cpp (L85-L92)
* It doesn't account for unicode encoding a string to bytes before taking its length.
The issues are mostly harmless edge cases, but it would be good to fix them. So do that here, by:
* Replacing `max()` by `sum()`, to correctly take into account all args, not just the largest one.
* Reduce `CLI_MAX_ARG_SIZE`, to account for the `bitcoin` command additional args.
Also, there is a test. The test can be called with `ulimit` to hopefully limit the max args size to the hard-coded value in the test framework. For reference:
```
$ ( ulimit -s 512 && python3 -c 'import os; print(os.sysconf("SC_ARG_MAX") )' )
131072
```
On top of this pull it should pass, ...
```
bash -c 'ulimit -s 512 && BITCOIN_CMD="bitcoin -M" ./bld-cmake/test/functional/rpc_misc.py --usecli -l DEBUG'
```
... and with the test_framework changes reverted, it should fail:
```
OSError: [Errno 7] Argument list too long: 'bitcoin'
```
Also, there is a commit to enable `CI_LIMIT_STACK_SIZE=1` in the i686 task, because it should now be possible and no longer hit the hard-to-reproduce issue mentioned above.
ACKs for top commit:
cedwies:
ACK fa96a4a
achow101:
ACK fa96a4afea
enirox001:
ACK fa96a4a — thanks for addressing the nits and clarifying the test; LGTM.
mzumsande:
Code Review ACK fa96a4afea
Tree-SHA512: d12211bd097d692d560c3615970ec0e911707d8c6cbbb145591abc548beed55f487a80b08f0a8c89d4eef4d76a9fbd6a33edc0b42b5860a93dd7b954355bc887
653a9849d5 common: Make arith_uint256 trivially copyable (Fabian Jahr)
Pull request description:
Makes `arith_uint256`/`base_uint` trivially copyable by removing the custom copy constructor and copy assignment operators. Removing of the custom code should not result in a change of behavior since `base_uint` contains a simple array of `uint32_t` and compiler generated versions of the code could be better optimized.
This was suggested by maflcko here: https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3186533494
ACKs for top commit:
Raimo33:
ACK 653a9849d5
l0rinc:
ACK 653a9849d5
achow101:
ACK 653a9849d5
hodlinator:
re-ACK 653a9849d5
Tree-SHA512: 38db5220a2cf773c0c5fb5591671e329b6b87458d972db4f5f3f98c025ec329a8c39b32b5bc24ef8b50b1002b43bb248d8b35aa1c9a56c68c6bbd1d470485bd7
75d9b72475 kernel: make blockTip index const (stickies-v)
Pull request description:
Notification interface subscribers need to view, but not mutate, the index.
This change allows improving the #30595 kernel interface, see e.g. `BlockTreeEntry` where [currently](https://github.com/bitcoin/bitcoin/pull/30595/files#diff-4d05cd02fdce641be603f0f9abcecfeaf76944285d4539ba4bbc40337fa9bbc2R617) a `View` is constructed from a non-const pointer, whereas really this should be a `const btck_BlockTreeEntry* entry`.
ACKs for top commit:
achow101:
ACK 75d9b72475
TheCharlatan:
ACK 75d9b72475
l0rinc:
Code review ACK 75d9b72475
yuvicc:
Code review ACK 75d9b72475
Tree-SHA512: 6151374a040cead36490c5fa5ce9dc4d93499a02110f444c50bd90f9095912747bc5b2fd7294815e6794c96a6843f43eb0507706d41d7296af96071b5f704ff4
fa4885ef2f test: Remove polling loop from test_runner (MarcoFalke)
Pull request description:
(This picks up my prior attempt from https://github.com/bitcoin/bitcoin/pull/13384)
Currently, the test_runner is using a `time.sleep` before polling to check if any tests have completed. This is largely fine when running a few tests, or when the tests take a long time.
However, when running many fast tests, this can accumulate and leave the CPU idle for no reason.
A trivial improvement would be to only sleep when really needed:
```diff
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 7c8c15f391..1d9f28cee4 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -747,7 +747,6 @@ class TestHandler:
dot_count = 0
while True:
# Return all procs that have finished, if any. Otherwise sleep until there is one.
- time.sleep(.5)
ret = []
for job in self.jobs:
(name, start_time, proc, testdir, log_out, log_err) = job
@@ -771,6 +770,7 @@ class TestHandler:
ret.append((TestResult(name, status, int(time.time() - start_time)), testdir, stdout, stderr, skip_reason))
if ret:
return ret
+ time.sleep(.5)
if self.use_term_control:
print('.', end='', flush=True)
dot_count += 1
```
However, ideally there is no sleep at all. So do that by using a `ThreadPoolExecutor`.
This can be tested via something like:
```
time ./bld-cmake/test/functional/test_runner.py $(for i in {1..200}; do echo -n "tool_rpcauth "; done) -j 200
```
The result should show:
* Current `master` is the slowest
* The "sleep patch" from above is a bit faster (1.5x improvement)
* This pull request is the fastest (2x improvement)
ACKs for top commit:
achow101:
ACK fa4885ef2f
l0rinc:
tested ACK fa4885ef2f
Eunovo:
ReACK fa4885ef2f
Tree-SHA512: f097636c5d9e005781012d8e20c2886cd9968544d4d555b1d2e28982d420ff63fec15cfabb6bd30e4d3c389b8b8350a1ddad721cceaf4b7760cad38b95160175
d45f3717d2 txgraph: use enum Level instead of bool main_only (Pieter Wuille)
Pull request description:
Part of #30289. Inspired by https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2331387778.
Since there has been more than one case in the development of #28676 of calling a `TxGraph` function without correctly setting the `bool main_only` argument that many of its interface functions have, make these mandatory and explicit, using an `enum class Level`:
```c++
enum class Level {
TOP, //!< Refers to staging if it exists, main otherwise.
MAIN //!< Always refers to the main graph, whether staging is present or not.
};
```
ACKs for top commit:
instagibbs:
ACK d45f3717d2
vasild:
ACK d45f3717d2
glozow:
code review ACK d45f3717d2
Tree-SHA512: d1c4b37e8ab3ec91b414df8970cb47aa080803f68da5881c8e1cbdc6939dea7851e0f715192cf3edd44b7f328cd6b678474d41f9cd9da8cb68f6c5fd78cb71b1
Replacing the custom code with default behavior should not result in a change of behavior since base_uint contains a simple array of uint32_t and compiler generated versions of the code could be better optimized.
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
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
4d4789dffa net: Prevent node from binding to the same CService (woltx)
Pull request description:
Currently, if the node inadvertently starts with repeated `-bind` options (e.g. `./build/bin/bitcoind -listen -bind=0.0.0.0 -bind=0.0.0.0`), the user will receive a misleading message followed by the node shutdown:
```
[net:error] Unable to bind to 0.0.0.0:8333 on this computer. Bitcoin Core is probably already running.
[error] Unable to bind to 0.0.0.0:8333 on this computer. Bitcoin Core is probably already running.
```
And the user might spend some time looking for a `bitcoind` process or what application is using port 8333, when what happens is that Bitcoin Core successfully connected to port 8333 and then tries again, generating this fatal error.
This PR proposes that repeated `-bind` options have no effect.
ACKs for top commit:
l0rinc:
ACK 4d4789dffa
yuvicc:
re-ACK 4d4789dffa
sipa:
utACK 4d4789dffa
achow101:
ACK 4d4789dffa
vasild:
ACK 4d4789dffa
naiyoma:
Tested ACK 4d4789dffa
Tree-SHA512: f1042c00417da16550403cfcb75cb8b12740e67cf92a1d8e3c007ae81fcf741907088a633129ce12a6a48ad07fc9f320602792cafed73ec33f6306cd854514b4
d3c5e47391 wallet, refactor: Remove Legacy check and error (pablomartin4btc)
30c6f64eed test: Remove unnecessary LoadWallet() calls (pablomartin4btc)
Pull request description:
Remove dead code due to legacy wallet removal.
Leftovers from previous #32481.
---
**Note**:
While attempting to remove the legacy check in `CWallet::UpgradeDescriptorCache()` (which is called from `DBErrors WalletBatch::LoadWallet(CWallet* pwallet))`, I once again ran into the fact that `LoadWallet()` is used in two distinct scenarios — something I was already aware of:
- Wallet creation – the upgrade is ignored here because no wallet flags are yet set; attempting to set a flag (ie `WALLET_FLAG_LAST_HARDENED_XPUB_CACHED` at the end of the upgrade function, if the legacy check is removed) would produce a failure (`DBErrors CWallet::LoadWallet()` -> `Assert(m_wallet_flags == 0)`).
- Wallet loading – the upgrade proceeds correctly and the flag `WALLET_FLAG_LAST_HARDENED_XPUB_CACHED` is set.
While revisiting this, I also noticed that some `LoadWallet()` calls in the wallet tests are unnecessary and I've removed them in the first commit.
The following change in `UpgradeDescriptorCache()` could be done in PR #32636 as part of the separation between wallet loading and creation responsibilities.
```diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
void CWallet::UpgradeDescriptorCache()
{
+ // Only descriptor wallets can upgrade descriptor cache
+ Assert(IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
+
- if (!IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS) || IsLocked() || IsWalletFlagSet(WALLET_FLAG_LAST_HARDENED_XPUB_CACHED)) {
+ if (IsLocked() || IsWalletFlagSet(WALLET_FLAG_LAST_HARDENED_XPUB_CACHED)) {
return;
}
```
ACKs for top commit:
davidgumberg:
crACK d3c5e47391
achow101:
ACK d3c5e47391
l0rinc:
code review ACK d3c5e47391
Tree-SHA512: ead37cf4061dfce59feb41ac50e807e6790e1a5e6b358e3b9c13e63d61a9cb82317a2e596cecb543f62f88a4338171788b651452425c1f40b5c1bec7fe78339e
53e6db91ef contrib: add placeholder manpage for bitcoin binary (fanquake)
f5887a8de4 contrib: add bitcoin binary to gen-manpages (fanquake)
Pull request description:
This was missed in #31375.
ACKs for top commit:
dergoegge:
ACK 53e6db91ef
Tree-SHA512: ff283ee02fadb57dbb335425d0348533b1322c6de323020f3ce5b6f01ff958cc731cb2191b8a774cd6a53b462f831e0ee86bbd522283357a6f6121962ef0abf1
9f744fffc3 build: bump CLIENT_VERSION_MAJOR to 30 (fanquake)
Pull request description:
Last step before branch off.
ACKs for top commit:
hebasto:
ACK 9f744fffc3.
Tree-SHA512: f8ddbaa56213707c4d1719a6ade89103bcc1142d71f47cc527a20669995c1598ddbd61a88487841aa794340219e956deed403d8a7c229fc8b526e67e07dd7d69
fa8f081af3 ci: Checkout latest merged pulls (MarcoFalke)
Pull request description:
Currently, the `actions/checkout@v5` checks out pull requests merged against master, which is what we want.
However, it checks out ancient/stale merge commits on a re-run. This is documented (https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs):
> Re-run workflows [...] will also use the same GITHUB_SHA (commit SHA) and GITHUB_REF (git ref) of the original event that triggered the workflow run.
For example:
* https://github.com/bitcoin/bitcoin/actions/runs/17458152407/job/49579638898?pr=29641#step:9:914 compiles with IPC=ON, even though latest master is at ed2ff3c63d
* https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3133536724 (example explained in comment)
This is problematic, because:
* Unrelated CI failures and intermittent issues, which are fixed or worked around in latest master can not be cleaned by re-running the task. The author has to actively go out and (force-)push the branch, invalidating review.
* It is odd to have a recent CI run, but it uses code and config from the past.
* Detecting silent merge conflicts by re-running the CI task is impossible.
Fix all issues by checking out the latest merged state of the pull request. The behavior is unchanged for non-pull-request actions. This patch changes the "re-run" default behaviour. Forcing it to use the new state instead of running the old state again.
ACKs for top commit:
janb84:
re ACK fa8f081af3
hebasto:
ACK fa8f081af3.
Tree-SHA512: c22c6f837402f61ec46be46817473e1946424b5312e36ed0e246cadb1ca89c04163bb471f71c309765a3d327f198a83cd83679d231f03828a99a97562a622fdd
5eeb2facbb ci: reduce runner sizes on various jobs (will)
Pull request description:
These jobs can likely use reduced runner sizes to avoid wasting our CPU quota, as much of the long-running part of the job is single-threaded.
This will also give us more (job) parallelisem from the same number of CPU that we are using.
Suggested in: https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2321775620
ACKs for top commit:
kevkevinpal:
ACK [5eeb2fa](5eeb2facbb)
m3dwards:
ACK 5eeb2facbb
janb84:
ACK 5eeb2facbb
Tree-SHA512: 6fb0352bc40623dd63b9bd6169d753d1ec9667c272445fda7a2db8bbedfa35350a51d08c1adf3fa5e070e84855c3f491668726d3c7ded07a39f2f9c63edacefc
Ehnamuram Enoch
merge-script
Ava Chow
Lőrinc
Ryan Ofsky
Martin Zumsande
merge-script
Hennadii Stepanov
Matthew Zipkin
TheCharlatan
Fabian Jahr
Pieter Wuille
David Gumberg