db3c25cfae index: add explicit early exit in NextSyncBlock() when the input is the chain tip (Hao Xu)
Pull request description:
When `pindex_prev` is the chain tip, `NextSyncBlock()` should return `nullptr` (there's no next block to sync). Currently this works, but relies on the fork/reorg path to produce the result: `chain.Next(tip)` returns `nullptr`, falling through to `FindFork(tip)` which returns `tip` itself, then `chain.Next(tip)` returns `nullptr` again.
Add an explicit early return for this case. This makes `NextSyncBlock()`'s three cases self-documenting:
1. next block exists on the active chain — return it
2. at the chain tip — return `nullptr`
3. on a stale branch — find the fork point and return the block after it
It also makes the existing comment ("Since block is not in the chain") accurate — the tip is in the chain, so it shouldn't reach that path.
ACKs for top commit:
optout21:
crACK db3c25cfae
furszy:
ACK db3c25cfae
stickies-v:
ACK db3c25cfae
Tree-SHA512: c1633bb3d3ffed2643c8e174c2b2283deaeffd0a06eb3fb2cf03eed097bfdc5cf6fbd7ebdcdb44fd56f712be8004ad46c730a33990fb1fc6cfa629c9b14f8cd9
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
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
70f632bda8f Merge bitcoin-core/libmultiprocess#265: ci: set LC_ALL in shell scripts
8e8e564259a Merge bitcoin-core/libmultiprocess#249: fixes for race conditions on disconnects
05d34cc2ec3 ci: set LC_ALL in shell scripts
e606fd84a8c Merge bitcoin-core/libmultiprocess#264: ci: reduce nproc multipliers
ff0eed1bf18 refactor: Use loop variable in type-context.h
ff1d8ba172a refactor: Move type-context.h getParams() call closer to use
1dbc59a4aa3 race fix: m_on_cancel called after request finishes
1643d05ba07 test: m_on_cancel called after request finishes
f5509a31fcc race fix: getParams() called after request cancel
4a60c39f24a test: getParams() called after request cancel
f11ec29ed20 race fix: worker thread destroyed before it is initialized
a1d643348f4 test: worker thread destroyed before it is initialized
336023382c4 ci: reduce nproc multipliers
b090beb9651 Merge bitcoin-core/libmultiprocess#256: ci: cache gnu32 nix store
be8622816da ci: cache gnu32 nix store
975270b619c Merge bitcoin-core/libmultiprocess#263: ci: bump timeout factor to 40
09f10e5a598 ci: bump timeout factor to 40
db8f76ad290 Merge bitcoin-core/libmultiprocess#253: ci: run some Bitcoin Core CI jobs
55a9b557b19 ci: set Bitcoin Core CI test repetition
fb0fc84d556 ci: add TSan job with instrumented libc++
0f29c38725b ci: add Bitcoin Core IPC tests (ASan + macOS)
3f64320315d Merge bitcoin-core/libmultiprocess#262: ci: enable clang-tidy in macOS job, use nullptr
cd9f8bdc9f0 Merge bitcoin-core/libmultiprocess#258: log: add socket connected info message and demote destroy logs to debug
b5d6258a42f Merge bitcoin-core/libmultiprocess#255: fix: use unsigned char cast and sizeof in LogEscape escape sequence
d94688e2c32 Merge bitcoin-core/libmultiprocess#251: Improved CustomBuildField for std::optional in IPC/libmultiprocess
a9499fad755 mp: use nullptr with pthread_threadid_np
f499e37850f ci: enable clang-tidy in macOS job
98f1352159d log: add socket connected info message and demote destroy logs to debug
554a481ea73 fix: use unsigned char cast and sizeof in LogEscape escape sequence
1977b9f3f65 Use std::forward in CustomBuildField for std::optional to allow move semantics, resolves FIXME
22bec918c97 Merge bitcoin-core/libmultiprocess#247: type-map: Work around LLVM 22 "out of bounds index" error
8a5e3ae6ed2 Merge bitcoin-core/libmultiprocess#242: proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values
e8d35246918 Merge bitcoin-core/libmultiprocess#246: doc: Bump version 8 > 9
97d877053b6 proxy-types: add CustomHasField hook for nullable decode paths
8c2f10252c9 refactor: add missing includes to mp/type-data.h
b1638aceb40 doc: Bump version 8 > 9
f61af487217 type-map: Work around LLVM 22 "out of bounds index" error
git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 70f632bda8f80449b6240f98da768206a535a04e
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
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
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
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
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
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
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.
0fe6fccec2 doc: Document rationale for using `IWYU pragma: export` (Hennadii Stepanov)
cfa3b10d50 iwyu, doc: Document `IWYU pragma: export` for `<logging/categories.h>` (Hennadii Stepanov)
015bea05e6 iwyu, doc: Document `IWYU pragma: export` for `<chrono>` (Hennadii Stepanov)
48bfcfedec iwyu, doc: Document `IWYU pragma: export` for `<threadsafety.h>` (Hennadii Stepanov)
179abb387f refactor: Move `StdMutex` to its own header (Hennadii Stepanov)
6d2952c3c3 serialize: Add missing `<span>` header (Hennadii Stepanov)
Pull request description:
This PR is a prerequisite for https://github.com/bitcoin/bitcoin/pull/34448. It was split into a separate PR to limit the scope and minimize potential merge conflicts.
The first commit improves the accuracy of IWYU suggestions within our heavily templated code. Note that, for now, the `serialize.h` header itself is excluded from IWYU inspection because it lacks a corresponding source file.
The remaining commits follow the Developer Notes [guidance](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#using-iwyu):
> Use `IWYU pragma: export` very sparingly, as this enforces transitive inclusion of headers and undermines the specific purpose of IWYU.
ACKs for top commit:
maflcko:
review ACK 0fe6fccec2👤
ajtowns:
utACK 0fe6fccec2
Tree-SHA512: dc2d4e3ff78b9707a1a26cb9b1c0a456de0d33c59e773bbf692344c2fceaff8936317479c5e898038f29134bc0e5d9d1ef7350e53512dd8e262f46ede578c4f9
0dc337f73d gui: Fix TransactionsView on setCurrentWallet (pablomartin4btc)
Pull request description:
<details>
<summary>Currenlty on <code>master</code>, when the "mask values" checkbox is ticked if the user selects a different wallet, the history action is enable and if the user clicks on it can see all the transactions in the transaction view.</summary>
</details>
<details>
<summary>This PR fixes it.</summary>
</details>
Note for maintainers: this needs to be backported to 25.x and 26.x.
ACKs for top commit:
hebasto:
ACK 0dc337f73d, tested on Fedora 43.
Tree-SHA512: 54581546917f87b4c1db0ff1eaa1962ee6eb078285dbb205b4c8d027c3e350f3dc46409b376948c10e668f9487b7a5a70bab0dff5faf510deab1a54452f7f0e5
The comparison is evaluated before the assignment, so total_amount
ends up holding a boolean instead of the actual amount:
total_amount = (a - b < c)
which is not what we want here. This has been working by accident.
The generateblock RPC creates blocks where the coinbase only includes
the block subsidy, omitting transaction fees. Document this behavior
in the RPC help text to avoid confusion.
refs #31684
74f71c5054 Remove Taproot activation height (Sjors Provoost)
Pull request description:
Drop `DEPLOYMENT_TAPROOT` from `consensus.vDeployments`.
Now that the commit (7c08d81e11) which changes taproot to be enforced for all blocks is included in v24.0, it seems a good time to remove no longer needed non-consensus code.
Clarify what is considered a `BuriedDeployment`.
We drop taproot from `getdeploymentinfo` RPC, rather than mark it as `buried`, because this is not a buried deployment in the sense of [BIP 90](https://github.com/bitcoin/bips/blob/master/bip-0090.mediawiki). This is because the activation height has been completely removed from the code, unlike the hardcoded `DEPLOYMENT_SEGWIT` height which is still relied on.[^1]
See discussion in #24737 and #26162.
[^1]: `DEPLOYMENT_SEGWIT` is used in `IsBlockMutated` to determine if witness data is allowed, it's used for [BIP147](https://github.com/bitcoin/bips/blob/master/bip-0147.mediawiki), to trigger `NeedsRedownload()` for users who upgraded after a decade, and for a few other things.
ACKs for top commit:
ajtowns:
reACK 74f71c5054
achow101:
ACK 74f71c5054
sedited:
Re-ACK 74f71c5054
darosior:
utACK 74f71c5054
Tree-SHA512: 217e0ee2e144ccfb04cf012f45b75d08f8541287a5531bd18aa81e38bad2f38d28b772137f471c46b63875840ec044cb61a2a832e3a9e89a6183e8ab36b1b9c9
a1f22a0a6b test: Suppress another unsolicited `mock_process/*` output (Hennadii Stepanov)
Pull request description:
This is a follow-up to https://github.com/bitcoin/bitcoin/pull/33929.
The[ `mock_process/*`](390e7d61bd/src/test/mock_process.cpp (L10)) test cases, which serve as helpers for [`system_tests`](390e7d61bd/src/test/system_tests.cpp (L23)) rather than actual tests, are invoked in a way that suppresses unsolicited output to stdout or stderr to keep the test results reproducible.
However, in debug builds, the Windows CRT still prints false-positive memory leak dumps to stderr.
This PR handles this specific case and documents the other suppressions.
ACKs for top commit:
maflcko:
lgtm ACK a1f22a0a6b
sedited:
ACK a1f22a0a6b
Tree-SHA512: 480f0f74ce50b6fb315bfeb6f6d3a4a4791557a6177aa995197c002bf871f878573df0f313344206f82e4d54c7a62d7cea2fdcb8cd96475b4b6cc7d4ffaf0538
faaea7895f refactor: Use current_time over redundant call to Now() (MarcoFalke)
3333c5023f refactor: Use NodeClock::time_point for m_addr_token_timestamp (MarcoFalke)
fa55723b8f move-only: Extract ProcessAddrs() helper (MarcoFalke)
Pull request description:
It is a bit confusing to have some code use the deprecated `GetTime`, which returns a duration and not a time point, and other code to use `NodeClock` time points.
Fix one place `m_addr_token_timestamp` to use `NodeClock::time_point`.
Also:
* Extract a `ProcessAddrs` helper, similar to the other `Process*()` helpers, to cut down the `ProcessMessage` with a massive scope.
* Rename the confusing `current_a_time` to `now_seconds`. (The `a` in this context refers to the removed "adjusted" time, see commit fadd8b2676, which removed adjusted time here)
ACKs for top commit:
l0rinc:
ACK faaea7895f
ajtowns:
reACK faaea7895f
sedited:
Re-ACK faaea7895f
Tree-SHA512: 67ad13e9d7b88e08e3d723e6b7cd598b38df2a004f5c2338b24f2992e25ae9d8fb8e5325c9c94171e551fe86d87e3e3ec1fe6baae64edbf6b5c125f408ee64e4
658e68f95b scripted-diff: Rename `WAIT_TIMEOUT` to `TEST_WAIT_TIMEOUT` (Hennadii Stepanov)
Pull request description:
On Windows, the `winerror.h` header defines `WAIT_TIMEOUT` as a macro.
This introduces a fragile dependency on header inclusion order: if Windows headers happen to be included before using `WAIT_TIMEOUT`, the preprocessor expands it into a numeric literal, causing syntax errors.
Rename the variable to `TEST_WAIT_TIMEOUT` to remove this fragility and avoid the collision entirely.
Split from https://github.com/bitcoin/bitcoin/pull/34448.
Similar to https://github.com/bitcoin/bitcoin/pull/34454.
ACKs for top commit:
w0xlt:
ACK 658e68f95b
Tree-SHA512: 90cf8927e4e41dee24d51fb2ea3335526ff5da4180c24d776d59b642794715b3c6558628088fbc28236d6ac4fffb9a27167fe309cb94ebf04f04c6e5cf957ad5
