890a09b1e4 fuzz: Use CAmount for storing best_waste (Ava Chow)
Pull request description:
Waste is a CAmount, which is an int64_t. This will overflow an int, so `best_waste` should also be a `CAmount`.
Fixes#34936
ACKs for top commit:
murchandamus:
ACK 890a09b1e4
furszy:
ACK 890a09b1e4
Tree-SHA512: c6c4f530960f038675d4549c2285c6a4a828099a631486e317ec1215d89688ce109304654a95800978607c360c2ed34803523f5c56ebf7c2324ca095f87825b8
65379bb8d0 ci: add FreeBSD cross CI job (fanquake)
f44191f163 depends: build qrencode for Freebsd (fanquake)
7f7018738e depends: FreeBSD cross with Clang (fanquake)
6464f14081 depends: disable inotify in Freebsd Qt build (fanquake)
Pull request description:
Alternative to #33562, which was adding a native FreeBSD job; however that had issues with permissions/caching, as well as potential determinism issues. This adds a FreeBSD cross job using Linux and Clang.
Would close#33438. The same changes here could also be used to produce FreeBSD binaries out of Guix.
ACKs for top commit:
hebasto:
ACK 65379bb8d0. I've cross-compiled on Ubuntu 25.10 for FreeBSD 14.4 and 15.0. The former binaries (`bitcoind`, `test_bitcoin` and `bitcoin-qt`) were tested on FreeBSD 14.4 locally.
Tree-SHA512: 52a3edaa56fe40ca901416cb9e1af04a84505526edfa7309bfa40024baa7d3b1a05303659553d9fbcf1f49d4e3d42b415a1e2523d448b22724d1415a49331259
f899674639 test: script: boundary at exactly 65535 bytes must use OP_PUSHDATA2 (Bruno Garcia)
Pull request description:
I noticed that the following mutant is not killed by any current test (can be tested with: `cmake --build build -j $(nproc) && ./build/bin/test_bitcoin && ./build/test/functional/test_runner.py -j $(nproc) -F`):
```diff
diff --git a/src/script/script.cpp b/src/script/script.cpp
index 3f764aaf21..5cff51d2cf 100644
--- a/src/script/script.cpp
+++ b/src/script/script.cpp
@@ -387,7 +387,7 @@ bool CheckMinimalPush(const std::vector<unsigned char>& data, opcodetype opcode)
} else if (data.size() <= 255) {
// Must have used OP_PUSHDATA.
return opcode == OP_PUSHDATA1;
- } else if (data.size() <= 65535) {
+ } else if (data.size() < 65535) {
// Must have used OP_PUSHDATA2.
return opcode == OP_PUSHDATA2;
}
```
This PR addresses it by adding a new test case to ensure that the boundary at exactly 65535 bytes must use OP_PUSHDATA2 as well.
ACKs for top commit:
kevkevinpal:
tACK [f899674](f899674639)
danielabrozzoni:
tACK f899674639
darosior:
utACK f899674639
w0xlt:
ACK f899674639
Tree-SHA512: ad35cc992aa351d26151cb79d1b1d5d960b1d80a98b3076a709aa19f7b5135edb87a957d2c84f359e86da8a15f7f0196301bfaff5ae554aecc65d81c97f8af3e
261d229455 test: Replace DEBUG_LOG_OUT with -printtoconsole=1 (Hodlinator)
Pull request description:
`-printtoconsole=1` has the same functionality but works in more cases than `DEBUG_LOG_OUT`, so remove the latter (`-printtoconsole` is already used when fuzzing). This means we can drop `G_TEST_LOG_FUN`.
---
Behavior can be verified through
```shell
ctest --test-dir build --debug
```
and checking for:
```
142: Test command: /home/hodlinator/bc/2/build/bin/test_bitcoin "--run_test=coinselector_tests" "--catch_system_error=no" "--log_level=test_suite" "--" "-printtoconsole=1"
...
142: 2026-03-26T13:40:02.169392Z [test] [../src/kernel/context.cpp:20] [operator()] Using the 'sse4(1way);sse41(4way);avx2(8way)' SHA256 implementation
```
---
Inspired by: https://github.com/bitcoin/bitcoin/pull/34918#discussion_r2994780532
ACKs for top commit:
nkatha23:
ACK 261d229
maflcko:
review ACK 261d229455📬
kevkevinpal:
crACK [261d229](261d229455)
janb84:
cr ACK 261d229455
Tree-SHA512: f4c793b4a00730ade113241baeaaef08ae0333df15ada5929dd46aacd1ac875733e58aa341fac8fb5875eb5ebca735d40c664bf0ef62132094e0c993da5b20e5
fa1ebde1ad fuzz: Use time helpers in node_eviction (MarcoFalke)
Pull request description:
The `node_eviction` fuzz test has many issues:
* It uses the full `int64_t` range (in seconds) as input, which is absurdly large (millions of years) and also violates https://en.cppreference.com/w/cpp/chrono/duration.html:
> Each of the predefined duration types up to hours covers a range of at least ±292 years.
* It does not use the existing `ConsumeDuration` and `ConsumeTime` helpers, which makes specifying a proper range difficult.
So fix all issues by using `ConsumeTime` for time points with default arguments, and `ConsumeDuration` with the same precision, as well as possibly negative values.
ACKs for top commit:
marcofleon:
crACK fa1ebde1ad
brunoerg:
reACK fa1ebde1ad
w0xlt:
ACK fa1ebde1ad
Tree-SHA512: 22045e6c563a9169327737895ea2f3a7b1dcb4fd24fce56d91caa1e132d03a85cbaaa5f78218d23cfa203fe2ee4b147894c02870eb20ae1c232ad55ccdb6f7f7
0d1301b47a test: functional: drop rmtree usage and add lint check (David Gumberg)
8bfb422de8 test: functional: drop unused --keepcache argument (David Gumberg)
a7e4a59d6d qa: Remove all instances of `remove_all` except test cleanup (David Gumberg)
Pull request description:
Both `fs::remove_all` and `shutil::rmtree()` are a bit dangerous, and most of their uses are not necessary, this PR removes most instances of both.
`remove_all()` is still used in in `src/test/util/setup_common.cpp` as part of `BasicTestingSetup::BasicTestingSetup`'s constructor and destructor, and it is used in the kernel test code's [`TestDirectory`](4ae00e9a71/src/test/kernel/test_kernel.cpp (L100-L112)):
734899a4c4/src/test/kernel/test_kernel.cpp (L100-L112)
In both cases, `remove_all` is likely necessary, but the kernel's test code is RAII, ideally `BasicTestingSetup` could be made similar in a follow-up or in this PR if reviewers think it is important.
Similarly in the python code, most usage was unnecessary, but there are a few places where `rmtree()` was necessary, I have added sanity checks to make sure these are inside of the `tmpdir` before doing recursive delete there.
ACKs for top commit:
achow101:
ACK 0d1301b47a
hodlinator:
ACK 0d1301b47a
sedited:
ACK 0d1301b47a
Tree-SHA512: da8ca23846b73eff0eaff61a5f80ba1decf63db783dcd86b25f88f4862ae28816fc9e2e9ee71283ec800d73097b1cfae64e3c5ba0e991be69c200c6098f24d6e
fa73ed467c refactor: Fix redundant conversion to std::string and then to std::string_view [performance-string-view-conversions] (MarcoFalke)
fa270fdacf refactor: Return std::optional from GetProxy (MarcoFalke)
faeac1a931 refactor: Return std::optional from GetNameProxy (MarcoFalke)
Pull request description:
Currently the getters have a mutable reference as inout param and return a bool to indicate success. This is confusing, because the success bool is redundant with the `IsValid()` state on the proxy object.
So in theory, the functions could reset the mutable proxy object to an invalid state and return `void`.
However, this would also be confusing, because devs can forget to check `IsValid()`.
Fix all issues by using `std::optional<Proxy>`, where devs no longer have to check `IsValid()` manually, or a separate bool. Note that new code in the repo is already using `std::optional<Proxy>`, see `git grep 'std::optional<Proxy>' bitcoin-core/master`. Also, `std::optional<Proxy>` will enforce checking at compile time, whereas calling `Proxy::IsValid` is not enforced.
ACKs for top commit:
achow101:
ACK fa73ed467c
sedited:
ACK fa73ed467c
ViniciusCestarii:
ACK fa73ed467c
frankomosh:
Code Review ACK fa73ed467c. Good refactor, correctly replaces the bool + mutable reference output parameter pattern with `std::optional<Proxy>` across `GetProxy` and `GetNameProxy`. Semantics are preserved.
Tree-SHA512: c6a1e1d1691958d2e6507e32e3484f96703fba03ccc710145ae2fb84b1254fb0e6e1d8d78e9b572daf5ea485247b73568704881762379b50bcf939a35494dd13
a49f97ff4a net: Don't log own ips during discover (sedited)
Pull request description:
The -logips option seems to imply that ip addresses are only logged when it is set. This seems like an obvious case for not logging these by default. There might be prior discussion around this, but I was unable to find why these might be exempt. There are also a few issues (both open and closed) where printing these lines was useful.
ACKs for top commit:
l0rinc:
tested ACK a49f97ff4a
achow101:
ACK a49f97ff4a
willcl-ark:
tACK a49f97ff4a
danielabrozzoni:
tACK a49f97ff4a
Tree-SHA512: 177911c5607b794965facf568fec71eb22fc8e9d16f4fee5088745e8aba51cb4ce839876c456470dc52839ebc36976dc6b81a50f546941aebb8db538d8fd4554
fabbfec3b0 fuzz: Remove unused g_setup pointers (MarcoFalke)
Pull request description:
This is unused and avoids a clang warning.
Can be tested via: `git grep --count '\<g_setup' | grep ':2'`
Before: Prints files names.
After: No output.
ACKs for top commit:
hebasto:
ACK fabbfec3b0.
hodlinator:
ACK fabbfec3b0
Tree-SHA512: 21a3459574316617aa98bb04e5b27173c4bb1e89d71ce1fb3fb3066f8d6e2b3bf1d1fd516a74f7ae3f9052b492edeec00d3365cb5781c5f79ebb828da7af520e
1438165b1f wallet: drop stale TODOs (Sjors Provoost)
Pull request description:
I don't remember why I added them. There's also no test that demonstrates the problem, or an open issue requesting a fix.
It's also unclear if this even _should_ be changed: https://github.com/bitcoin/bitcoin/pull/34912#issuecomment-4121780033
ACKs for top commit:
polespinasa:
ACK 1438165b1f
kevkevinpal:
ACK [1438165](1438165b1f)
w0xlt:
ACK 1438165b1f
Tree-SHA512: 5b6024f38222444488ea3892d1b32277a4091d9c17bc4e8d54efda4d8b8ec567fe639b07fe5bfb26fc9cb8a783bac455dd143f1d731ce111449c9a3aec13368d
These are unused and removing them avoids clang warnings like:
src/test/fuzz/deserialize.cpp:42:26: error: variable g_setup set but not used [-Werror,-Wunused-but-set-variable]
3129d4a693 ci: Rename `TIDY_LLVM_V` to `IWYU_LLVM_V` in IWYU-specific code (Hennadii Stepanov)
0b489886f8 ci: Upgrade IWYU to 0.26 compatible with Clang 22 (Hennadii Stepanov)
Pull request description:
This PR upgrades IWYU to [0.26](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.26) and removes mapping workarounds for issues that have been fixed upstream.
ACKs for top commit:
fanquake:
ACK 3129d4a693
Tree-SHA512: 9a926e489573d040423461c039ecda7636beb70e8214a02c1c594cbd5b89d26331455a9872c38993fa5ee6d27fdfefc2375dedc369721b2933411542a57a3884
`shutil.rmtree` is dangerous because it recursively deletes. There are
not likely to be any issues with it's current uses, but it is possible
that some of the assumptions being made now won't always be true, e.g.
about what some of the variables being passed to `rmtree` represent.
For some remaining uses of rmtree that can't be avoided for now, use
`cleanup_dir` which asserts that the recursively deleted folder is a
child of the the `tmpdir` of the test run. Otherwise,
`tempfile.TemporaryDirectory` should be used which does it's own
deleting on being garbage collected, or old fashioned unlinking and
rmdir in the case of directories with known contents.
At the time this was added in #10197, building the test cache took 21
seconds, as described in that PR, but this is no longer true, as
demonstrated by running the functional test framework with and without
the --keepcache arguments on master prior to this commit:
```
hyperfine --warmup 1 --export-markdown results.md --runs 3 \
-n 'without --keepcache' './build/test/functional/test_runner.py -j $(nproc)' \
-n 'with --keepcache' './build/test/functional/test_runner.py -j $(nproc) --keepcache'
```
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|:----------------------|---------------:|--------:|---:|---:|
| `without --keepcache` | 76.373 ± 3.058 | 74.083 | 79.846 | 1.00 |
| `with --keepcache` | 77.384 ± 1.836 | 75.952 | 79.454 | 1.01 ± 0.05 |
As a consequence, this argument can be removed from the test runner and
this also has the benefit of being able to use an RAII-like
`tempfile.TemporaryDirectory` instead of having to clean up the cache
manually at the end of test runs.
bitcoin/bitcoin#10197: https://github.com/bitcoin/bitcoin/pull/10197
Adds a lint check for `remove_all()`
`fs::remove_all()`/`std::filesystem::remove_all()` is extremely
dangerous, all user-facing instances of it have been removed, and it
also deserves to be removed from the places in our test code where it is
being used unnecessarily.
69baddc910 validation: do not add the snapshot block to candidates of bg chainstate (Martin Zumsande)
Pull request description:
The snapshot block needs to be added to the candidates set of the assumed-valid chain because it will be the tip of that chainstate right after snapshot activation.
However, adding it also to the background chainstate is not necessary for anything. Before, the index would be in the set without being connectable. It will be eventually added to the set as part of the normal block download - no extra logic is necessary here.
This simplifies a unit test which had a comment that having the block in the set is "not intended".
This was suggested [here](https://github.com/bitcoin/bitcoin/pull/34521#discussion_r2849281299) and [here](https://github.com/bitcoin/bitcoin/pull/34521#discussion_r2883024248) in #34521
Note that adding the snapshot block was harmless, since `FindMostWorkChain()` lazily removes blocks without data from the set, so this does not fix a bug but just simplifies some code.
ACKs for top commit:
achow101:
ACK 69baddc910
Bortlesboat:
Concept ACK 69baddc910. Removing `TargetBlock()` correctly limits the snapshot-block special-case to cs2 where it's actually needed — the test's own "not intended" comment was the tell.
sedited:
ACK 69baddc910
fjahr:
ACK 69baddc910
stratospher:
ACK 69baddc.
Tree-SHA512: 8942fc422f1898369dd486e37da11758f2ebd4a488d092aa1637ef5bfb85766c4be9ad0718797fb2080f5e8d61383b2ee932bf2bc2f7abc2fb07fe3d72e070c3
25f69d970a release note (Pol Espinasa)
af629821cf test: add background validation test for getblockchaininfo (Pol Espinasa)
a3d6f32a39 rpc, log: add backgroundvalidation to getblockchaininfo (Pol Espinasa)
5b2e4c4a88 log: update progress calculations for background validation (Pol Espinasa)
Pull request description:
`getblockchaininfo` returns `verificationprogress=1` and `initialblockdownload=false` even if there's background validation.
This PR adds information about background validation to rpc `getblockchaininfo` in a similar way to `validationprogress` does.
If assume utxo was used the output of a "sync" node performing background validation:
```
$ ./build/bin/bitcoin-cli getblockchaininfo
...
"mediantime": 1756933740,
"verificationprogress": 1,
"initialblockdownload": false,
"backgroundvalidation": {
"snapshotheight": 880000,
"blocks": 527589,
"bestblockhash": "0000000000000000002326308420fa5ccd28a9155217f4d1896ab443d84148fa",
"mediantime": 1529076654,
"chainwork": "0000000000000000000000000000000000000000020c92fab9e5e1d8ed2d8dbc",
"verificationprogress": 0.2815790617966284
},
"chainwork": "0000000000000000000000000000000000000000df97866c410b0302954919d2",
"size_on_disk": 61198817285,
...
```
If assume utxo was not used the progress is hidden:
```
$ ./build/bin/bitcoin-cli getblockchaininfo
...
"mediantime": 1756245700,
"verificationprogress": 1,
"initialblockdownload": false,
"chainwork": "00000000000000000000000000000000000000000000000000000656d6bb052b",
"size_on_disk": 3964972194,
...
```
The PR also updates the way we estimate the verification progress returning a 100% on the snapshot block and not on the tip as we will stop doing background validation when reaching it.
ACKs for top commit:
fjahr:
ACK 25f69d970a
danielabrozzoni:
ACK 25f69d970a
achow101:
ACK 25f69d970a
sedited:
ACK 25f69d970a
Tree-SHA512: 5e5e08fd39af5f764962b862bc6d8257b0d2175fe920d4b79dc5105578fd4ebe08aee2fe9bfa5c9cad5d7610197a435ebaac0de23e7a5efa740dfea031a8a9d4
3136559923 doc: Note that generateblock does not collect transaction fees (HouseOfHufflepuff)
Pull request description:
## Summary
- Add a note to the `generateblock` RPC help text clarifying that transaction fees are not collected in the block reward
- This was suggested by maflcko in #31684 as a minimal doc fix
refs #31684
## Test plan
- [x] `./build/bin/test_bitcoin --run_test=rpc_tests` passes
- [x] `cmake --build build --target bitcoind` compiles cleanly
ACKs for top commit:
maflcko:
lgtm ACK 3136559923
sedited:
ACK 3136559923
Tree-SHA512: 173e6ac2a08c5101794a21bf29ec01af834fe2ef177b46be9f46b0c545b8888a9deb816caed868ff917105cbd97e57152682dcd4d4fe47dd92ac14a83bbf5d03
ad75b147b5 test: scale IPC mining wait timeouts by timeout_factor (Enoch Azariah)
e7a918b69a test: verify IPC error handling for invalid coinbase (Enoch Azariah)
63684d6922 test: move make_mining_ctx to ipc_util.py (Enoch Azariah)
4ada575d6c test: verify createNewBlock wakes promptly when tip advances (Enoch Azariah)
Pull request description:
This is a follow-up to implement a couple of test improvements discussed in recent IPC PRs and issues.
- adds a test to `interface_ipc_mining.py` to verify that `createNewBlock` wakes up immediately when the tip advances, rather than waiting for the cooldown timer to expire (https://github.com/bitcoin/bitcoin/pull/34184#discussion_r2842239399).
- moves `make_mining_ctx` into `ipc_util.py` so it can be reused across the IPC tests instead of duplicating the setup code (https://github.com/bitcoin/bitcoin/pull/34422#discussion_r2852445430).
- adds a test case to verify that providing an invalid coinbase to `submitSolution` returns a remote exception instead of crashing the node, closing the loop on the issue reported in #33341.
- scales IPC wait timeouts using the test suite's `timeout_factor` to prevent spurious failures in heavily loaded CI environments, capping extended waits to avoid test runner hangs (bitcoin-core/libmultiprocess#253 (comment)).
Closes#33341.
ACKs for top commit:
Sjors:
ACK ad75b147b5
achow101:
ACK ad75b147b5
sedited:
ACK ad75b147b5
Tree-SHA512: 812aa64c03657761f06707e6a15b8b435ab7c75717a6748a919fcbcae317128e18403b0c1bddd4cdad877d286e69db52389633e4012faaa656acc01939091719
353c660be5 bench: use deterministic `HexStr` payload (Lőrinc)
Pull request description:
### Context
Split out of https://github.com/bitcoin/bitcoin/pull/32554
Inspired by https://github.com/bitcoin/bitcoin/pull/32457#discussion_r2081323234
### Problem
`HexStrBench` uses the bytes from the embedded block fixture as a random source of bytes to measure `HexStr` performance against.
This coupling makes block benchmark migrations in #32554 slightly more work than necessary.
### Fix
We can use deterministic pseudo-random bytes instead so this benchmark keeps stable input without fixture coupling.
Use `MAX_BLOCK_WEIGHT` so the benchmark stays in the same size range and keeps measured work above harness overhead.
This changes the benchmark baseline because input size moves from about 1 MB to 4 MB.
ACKs for top commit:
maflcko:
lgtm ACK 353c660be5
sedited:
ACK 353c660be5
hodlinator:
ACK 353c660be5
Tree-SHA512: 5144b9b71761c581ff13c8f1163efed5e97f247f3c760dd7e513209c84d50f13253aa0d2ab3a431aaa51c204fea51bece41ac128b3d0e3a9ef02d1be11d6a6db
8d2f06853a sync: Use StdMutex for thread safety annotations (Anthony Towns)
cbc231ed8e scripted-diff: logging: Switch from StdLockGuard to STDLOCK (Anthony Towns)
f808786f48 logging: Add missing thread safety annotations (Anthony Towns)
e196cf26e0 util/stdmutex.h: Add STDLOCK() and improve annotation checking for StdMutex (Anthony Towns)
Pull request description:
Using `STDLOCK(mutex)` instead of `StdLockGuard guard(mutex)` allows clang to propagate missing lock annotations backwards (for global locks or locks in the same class, anyway), and also avoids declaring a dummy name. Use this in logging.h, and also use it in sync.cpp, adding annotations around the internal structure.
ACKs for top commit:
theuni:
ACK 8d2f06853a
sedited:
ACK 8d2f06853a
Tree-SHA512: ee23f6a7bcc62cc6d9ea88afa863a9018e53a0932272bb14241441fb69066c6633c4e19aadd3dd7599848d4742099d63756be4eff1969775293b728f3dd187aa
fa30951af5 test: Remove confusing assert_debug_log in wallet_reindex.py (MarcoFalke)
Pull request description:
The `wallet_reindex.py` test has many issues:
* It uses a `assert_debug_log` to ensure the reindex happened via `initload thread exit`. However, no other test uses this pattern, because `restart_node` already achieves the same internally (via `getmempoolinfo.loaded`).
* The `assert_debug_log` may intermittently fail, because the `initload thread exit` log may happen at any time after the inner thread function (lambda) is fully done.
* The test uses an `node.syncwithvalidationinterfacequeue()` without any explanation. No other test uses this pattern, because all wallet RPCs, in particular the next one (`gettransaction`) call it internally (via `BlockUntilSyncedToCurrentChain`).
Fix all issues by removing all confusing, useless, and brittle calls.
Example failure: https://github.com/bitcoin/bitcoin/actions/runs/22671604602/job/65716917459?pr=34419#step:11:22339
```
...
node0 2026-03-04T13:55:23.784715Z (mocktime: 2026-03-04T22:15:17Z) [scheduler] [validationinterface.cpp:185] [operator()] [validation] UpdatedBlockTip: new block hash=24b5cc4c1352bc8841ecbe67a43827acd1adc609bd26b2691e80e89b97eff135 fork block hash=24a4dc8be9c157ac31913397d8bb1653900e12b55539c234039559fdeb6dd2fb (in IBD=true)
node0 2026-03-04T13:55:23.784851Z (mocktime: 2026-03-04T22:15:17Z) [initload] [util/thread.cpp:22] [TraceThread] initload thread exit
test 2026-03-04T13:55:23.813824Z TestFramework (ERROR): Unexpected exception:
Traceback (most recent call last):
File "/home/admin/actions-runner/_work/_temp/test/functional/test_framework/test_framework.py", line 142, in main
self.run_test()
File "/home/admin/actions-runner/_work/_temp/build/test/functional/wallet_reindex.py", line 90, in run_test
self.birthtime_test(node, miner_wallet)
File "/home/admin/actions-runner/_work/_temp/build/test/functional/wallet_reindex.py", line 64, in birthtime_test
with node.assert_debug_log(expected_msgs=["initload thread exit"]):
File "/usr/lib/python3.12/contextlib.py", line 144, in __exit__
next(self.gen)
File "/home/admin/actions-runner/_work/_temp/test/functional/test_framework/test_node.py", line 607, in assert_debug_log
self._raise_assertion_error(f'Expected message(s) {remaining_expected!s} '
File "/home/admin/actions-runner/_work/_temp/test/functional/test_framework/test_node.py", line 241, in _raise_assertion_error
raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Expected message(s) ['initload thread exit'] not found in log:
```
Diff to reproduce failure:
```diff
diff --git a/src/util/thread.cpp b/src/util/thread.cpp
index 0fde73c4e2..de4c05e752 100644
--- a/src/util/thread.cpp
+++ b/src/util/thread.cpp
@@ -8,2 +8,3 @@
#include <util/log.h>
+#include <util/time.h>
#include <util/threadnames.h>
@@ -21,2 +22,3 @@ void util::TraceThread(std::string_view thread_name, std::function<void()> threa
thread_func();
+ UninterruptibleSleep(999ms);
LogInfo("%s thread exit", thread_name);
diff --git a/test/functional/wallet_reindex.py b/test/functional/wallet_reindex.py
index 71ab69e01b..649df4301a 100755
--- a/test/functional/wallet_reindex.py
+++ b/test/functional/wallet_reindex.py
@@ -62,2 +62,3 @@ class WalletReindexTest(BitcoinTestFramework):
+ import time; time.sleep(1) # Wait for previous initload thread to exit fully
# Reindex and wait for it to finish
```
Before on master: Test fails
After on this pull: Test passes
ACKs for top commit:
rkrux:
ACK fa30951af5 if this PR needs to be backported.
Tree-SHA512: 1e6599511e09b5b9ac4aed96a6b1b8239d5dc63b164fc0c163b6f9f5bc72c1c61f72ad8256a01875208d825af46d057c4d9de61758002f256388ddb324006bb5
6072a2a6a1 wallet: feebumper, fix crash when combined bump fee is unavailable (furszy)
Pull request description:
When a large cluster of unconfirmed transactions exceeds the limit,
`calculateCombinedBumpFee()` returns `std::nullopt`.
Previously, we continued executing and the optional value was
accessed unconditionally, leading to a `std::bad_optional_access`
exception (https://en.cppreference.com/w/cpp/utility/optional/value.html).
Fix this by returning early when the bumped fee is null.
Note:
This is a crash for the GUI, and an uncaught exception for the RPC
`bumpfee` and `psbtbumpfee`.
ACKs for top commit:
achow101:
ACK 6072a2a6a1
luke-jr:
utACK 6072a2a6a1
rkrux:
crACK 6072a2a6a1 based on returning before accessing the null optional.
Tree-SHA512: f863ace1426b2e743e2281e5c624b523de7317c1f305f88f369e77d60005460e4af58b424bc784304fd1ac30a3bfa575137537ec334fa6e449c827daeb262a99
92287ae753 test/wallet: ensure FastWalletRescanFilter is updated during scanning (Novo)
Pull request description:
Part of https://github.com/bitcoin/bitcoin/pull/34400
On master, the `wallet_fast_rescan.py` test will pass even if the fast rescan filters are not updated during wallet rescan. This PR improves the `wallet_fast_rescan.py` test so that this no longer happens. Testers can verify by applying this diff
```diff
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 63dab29972..f7d6c5f84a 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1898,7 +1898,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
bool fetch_block{true};
if (fast_rescan_filter) {
- fast_rescan_filter->UpdateIfNeeded();
+ // fast_rescan_filter->UpdateIfNeeded();
auto matches_block{fast_rescan_filter->MatchesBlock(block_hash)};
if (matches_block.has_value()) {
if (*matches_block) {
```
and running the `wallet_fast_rescan.py` test. The test will pass on master but fail on this PR.
ACKs for top commit:
achow101:
ACK 92287ae753
Bortlesboat:
re-ACK 92287ae753
rkrux:
tACK 92287ae
ismaelsadeeq:
Tested ACK 92287ae753
Tree-SHA512: bcd66db0be6604b084230fa3d3aa8d99f5a1a679a7a92592a5e8962abbcf35a14fb6883f25c9e8e6c4799893b774b1c6766876587417f35c4190562ef5ad39fb
0026b330c4 wallet: fix amount computed as boolean in coin selection (furszy)
Pull request description:
Stumbled upon this tiny bug. This has been working by accident.
The comparison is evaluated first, so `total_amount` ends up holding a boolean instead of the actual amount.
It can be verified by adding the value to the returned error message and running the `wallet_create_tx.py`
test.
Note:
I assume something like `-Wparentheses` in CI (or similar) should help us catching similar issues elsewhere.
ACKs for top commit:
fjahr:
utACK 0026b330c4
achow101:
ACK 0026b330c4
andrewtoth:
utACK 0026b330c4
luke-jr:
utACK 0026b330c4
Tree-SHA512: 289c1eb34e59caae0a9e6814a14e4a7ba72f26e3b26717bb3f9e60335c9c5efcebe7e5997f799752096cf91df416a82b8677a9900e5ec54b6d13921d4299be96
b149a28f6b depends: Do not consider `CC` environment variable when detecting system (Hennadii Stepanov)
Pull request description:
On the master branch @ 3c88eac28e, consider the following commands in the `depends` subdirectory:
```sh
$ make print-build HOST=i686-pc-linux-gnu CC="clang -m32"
build=x86_64-pc-linux-gnu
$ make print-host HOST=i686-pc-linux-gnu CC="clang -m32"
host=i686-pc-linux-gnu
```
The printed variable values are expected.
However, switching the `CC` variable context from Makefile to the shell environment breaks expectations:
```sh
$ CC="clang -m32" make print-build HOST=i686-pc-linux-gnu
build=i686-pc-linux-gnu
$ CC="clang -m32" make print-host HOST=i686-pc-linux-gnu
host=i686-pc-linux-gnu
```
This PR fixes this issue.
#### UPDATE 2026-01-20
On the master branch @ 7f5ebef56a:
```
$ gmake print-build HOST=i686-pc-linux-gnu CC="clang -m32"
build=i686-pc-linux-gnu
$ gmake print-host HOST=i686-pc-linux-gnu CC="clang -m32"
host=i686-pc-linux-gnu
```
ACKs for top commit:
sedited:
Re-ACK b149a28f6b
ryanofsky:
Code review ACK b149a28f6b. `env --unset` was replaced with `unset &&` since last review. I still think it could be better to write `CC=$(build_CC)` here even if `build_CC` may not be defined at this point (https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-3774229317) because it makes intent of the code more obvious, but current PR is already an improvement
Tree-SHA512: bd498706ad46aab93192e21b7cc30c34a714c5f31601122752fc94416dc51846b8d4eaf5d1c3250ba64d4eadf3fd43c9f7f5d9e178a47ee2165474ac6833fa31
c68e3d2c57 doc: add release notes for Tor PoW defenses (Vasil Dimov)
4bae84c94a doc: add a hint to enable PoW defenses to manual hidden services (Vasil Dimov)
4c6798a3d3 tor: enable PoW defenses for automatically created hidden services (Vasil Dimov)
fb993f7604 tor, fuzz: reuse constants instead of duplicating (Vasil Dimov)
Pull request description:
Enable [PoW defenses](https://tpo.pages.torproject.net/onion-services/ecosystem/technology/security/pow/) for hidden services that we create via Tor Control using the [`ADD_ONION` command](https://spec.torproject.org/control-spec/commands.html#add_onion).
The ability to do that has been added in [tor-0.4.9.2-alpha](02c1804446). Previous versions return a syntax error to the `ADD_ONION` command with `PoWDefensesEnabled=1`, so the approach here is to try with PoW and if we get syntax error, then retry without PoW.
Also update `doc/tor.md` with a hint on enabling PoW on manually configured Tor hidden services.
ACKs for top commit:
willcl-ark:
ACK c68e3d2c57
fjahr:
tACK c68e3d2c57
sedited:
ACK c68e3d2c57
Tree-SHA512: 56f57bc770b89389c35a4c0bc2a28804d17b1479ecd4d9b764695d6c9d2994425aee759e71658d7b57088bbe43ce90b94fb972f66d79ef903e0c1a4d6c4562f3
StdLockGuard and clang's thread safety annotations did not ensure that
the lock it was taking was not already held. Add a STDLOCK() macro which
uses an annotated StdMutex::CheckNotHeld() function to correct that.
