78358ce09d ci: add libzmq3-dev to test-each-commit job (fanquake)
a2b1d2c5ec doc: remove bsdmainutils (fanquake)
36ff336d2e ci: remove bsdmainutils (fanquake)
Pull request description:
This was previously used to install `hexdump` (for the tests). However that isn't used by CMake. I'm not aware of any other tools from this package being used.
ACKs for top commit:
maflcko:
ACK 78358ce09d
hebasto:
ACK 78358ce09d.
Tree-SHA512: 01c1be81feba03a9645e3d382067df4cd7c64de184871c8d9691053a8f871fcedf48d298303554560df2cb1949fb35d5ce9ff20c751ff35789b7689d656c0287
fac587ea07 ci: Use C++23 once for testing (MarcoFalke)
fa053ab7c0 build: Add Centos Stream 9 EOL URL (MarcoFalke)
Pull request description:
There are no plans to switch to C++23 anytime soon in the next couple of years. The only place right now that is known to benefit is `src/compat/byteswap.h`.
However, it is still useful to test with the option, because deprecated, removed or changed language features, as well as compiler changes that are guarded by the language version will be tested and developers can learn about them upfront.
Also includes a minor doc fixup commit.
ACKs for top commit:
davidgumberg:
ACK fac587ea07
TheCharlatan:
ACK fac587ea07
Tree-SHA512: 1b81788eb5b4da77715d8b047279de65ae6b8920d5a21fd8cc94c3b0edb588ab8ffb7eaffb2f8b7806045de1d47ca85ca629f49038eca762f3136bf380cf3c87
7de0c99804 doc: update dev note examples for CMake (fanquake)
Pull request description:
Update the examples in the developer notes to work with CMake.
Also added an explicit `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON` for clarity.
ACKs for top commit:
davidgumberg:
Tested ACK 7de0c99804
TheCharlatan:
ACK 7de0c99804
jonatack:
Tested ACK 7de0c99804 on arm64 macOS 14.6.1
Tree-SHA512: 561fe5e777c5b29a4f26309700c03a730c5bbb2f838630abfaa4174112ced66e733c2109cb429a1927f1f3692bf1945f6386bcaffe604a76ea24633932d39171
fa80d39d82 ci: Re-add configs removed in cmake migration (MarcoFalke)
Pull request description:
In commit 9730288a0c many configs were removed from the CI without explanation.
Fix it by adding them back.
Can be reviewed by looking at:
* the parity table https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e
* the installed packages
* the CI logs from before the cmake migration and the CI logs of this pull request
ACKs for top commit:
fanquake:
ACK fa80d39d82
Tree-SHA512: a33335e117750e6c2e1490bb621f67c466f901793e43abe1bd0e263ef16fdcbc9e88be55c206167f3a5ddb39c1df6989c0fb7a96d9240243c000ba2e7f5e2747
7ee5c3c5b2 Fix a few likely documentation typos (Lőrinc)
Pull request description:
Found them during CMake migration - and ran a quick spellcheck for the rest to cover any remaining ones
ACKs for top commit:
maflcko:
lgtm ACK 7ee5c3c5b2
Tree-SHA512: c6e7aa1e952e0d093745c4e6004c3907b7a215c6f998cc205307c0c68abcc067bf3f56e22af0deb1710186e8a871306f4bae8a35c74581e5299abcbbcddfaa75
948238a683 test: Remove FastRandomContext global (Ryan Ofsky)
fa0fe08eca scripted-diff: [test] Use g_rng/m_rng directly (MarcoFalke)
fa54cab473 test: refactor: Accept any RandomNumberGenerator in RandMoney (MarcoFalke)
68f77dd21e test: refactor: Pass rng parameters to test functions (Ryan Ofsky)
fa19af555d test: refactor: Move g_insecure_rand_ctx.Reseed out of the helper that calls MakeRandDeterministicDANGEROUS (MarcoFalke)
3dc527f460 test: refactor: Give unit test functions access to test state (Ryan Ofsky)
fab023e177 test: refactor: Make unsigned promotion explicit (MarcoFalke)
fa2cb654ec test: Add m_rng alias for the global random context (MarcoFalke)
fae7e3791c test: Correct the random seed log on a prevector test failure (MarcoFalke)
Pull request description:
This is mostly a style-cleanup for the tests' random generation:
1) `g_insecure_rand_ctx` in the tests is problematic, because the name is a leftover when the generator was indeed insecure. However, now the generator is *deterministic*, because the seed is either passed in or printed (c.f. RANDOM_CTX_SEED). Stating that deterministic randomness is insecure in the tests seems redundant at best. Fix it by just using `m_rng` for the name.
2) The global random context has many one-line aliases, such as `InsecureRand32`. This is problematic, because the same line of code may use the context directly and through a wrapper at the same time. For example in net_tests (see below). This inconsistency is harmless, but confusing. Fix it by just removing the one-line aliases.
```
src/test/net_tests.cpp: auto msg_data_1 = g_insecure_rand_ctx.randbytes<uint8_t>(InsecureRandRange(100000));
````
3) The wrapper for randmoney has the same problem that the same unit test uses the context directly and through a wrapper at the same time. Also, it has a single type of Rng hardcoded. Fix it by accepting any type.
ACKs for top commit:
hodlinator:
ACK 948238a683
ryanofsky:
Code review ACK 948238a683. Only changes since last review were changing a comments a little bit.
marcofleon:
Code review ACK 948238a683. Only changes since my last review are the improvements in `prevector_tests`.
Tree-SHA512: 69c6b46a42cb743138ee8c87ff26a588dbe083e3efb3dca49b8a133ba5d3b09e8bf01c590ec7e121a7d77cb1fd7dcacd927a9ca139ac65e1f7c6d1ec46f93b57
a0abcbd382 doc: Mention multipath specifier (Ava Chow)
0019f61fc5 tests: Test importing of multipath descriptors (Ava Chow)
f97d5c137d wallet, rpc: Allow importdescriptors to import multipath descriptors (Ava Chow)
32dcbca3fb rpc: Allow importmulti to import multipath descriptors correctly (Ava Chow)
64dfe3ce4b wallet: Move internal to be per key when importing (Ava Chow)
1692245525 tests: Multipath descriptors for scantxoutset and deriveaddresses (Ava Chow)
cddc0ba9a9 rpc: Have deriveaddresses derive receiving and change (Ava Chow)
360456cd22 tests: Multipath descriptors for getdescriptorinfo (Ava Chow)
a90eee444c tests: Add unit tests for multipath descriptors (Ava Chow)
1bbf46e2da descriptors: Change Parse to return vector of descriptors (Ava Chow)
0d640c6f02 descriptors: Have ParseKeypath handle multipath specifiers (Ava Chow)
a5f39b1034 descriptors: Change ParseScript to return vector of descriptors (Ava Chow)
0d55deae15 descriptors: Add DescriptorImpl::Clone (Ava Chow)
7e86541f72 descriptors: Add PubkeyProvider::Clone (Ava Chow)
Pull request description:
It is convenient to have a descriptor which specifies both receiving and change addresses in a single string. However, as discussed in https://github.com/bitcoin/bitcoin/issues/17190#issuecomment-895515768, it is not feasible to use a generic multipath specification like BIP 88 due to combinatorial blow up and that it would result in unexpected descriptors.
To resolve that problem, this PR proposes a targeted solution which allows only a single pair of 2 derivation indexes to be inserted in the place of a single derivation index. So instead of two descriptor `wpkh(xpub.../0/0/*)` and `wpkh(xpub.../0/1/*)` to represent receive and change addresses, this could be written as `wpkh(xpub.../0/<0;1>/*)`. The multipath specifier is of the form `<NUM;NUM>`. Each `NUM` can have its own hardened specifier, e.g. `<0;1h>` is valid. The multipath specifier can also only appear in one path index in the derivation path.
This results in the parser returning two descriptors. The first descriptor uses the first `NUM` in all pairs present, and the second uses the second `NUM`. In our implementation, if a multipath descriptor is not provided, a pair is still returned, but the second element is just `nullptr`.
The wallet will not output the multipath descriptors (yet). Furthermore, when a multipath descriptor is imported, it is expanded to the two descriptors and each imported on its own, with the second descriptor being implicitly for internal (change) addresses. There is no change to how the wallet stores or outputs descriptors (yet).
Note that the path specifier is different from what was proposed. It uses angle brackets and the semicolon because these are unused characters available in the character set and I wanted to avoid conflicts with characters already in use in descriptors.
Closes#17190
ACKs for top commit:
darosior:
re-ACK a0abcbd382
mjdietzx:
reACK a0abcbd382
pythcoiner:
reACK a0abcbd
furszy:
Code review ACK a0abcbd
glozow:
light code review ACK a0abcbd382
Tree-SHA512: 84ea40b3fd1b762194acd021cae018c2f09b98e595f5e87de5c832c265cfe8a6d0bc4dae25785392fa90db0f6301ddf9aea787980a29c74f81d04b711ac446c2
04fb085f6b build: remove old MSVC build system (fanquake)
Pull request description:
This has been superseded by CMake, and should not be blocked on anything for removal.
ACKs for top commit:
laanwj:
ACK 04fb085f6b
hebasto:
ACK 04fb085f6b.
TheCharlatan:
ACK 04fb085f6b
vasild:
ACK 04fb085f6b, except:
Tree-SHA512: 219005bd0de1a9e37dabe45c979601df3083368f4c5ea224f70cf4e6f52e49d8f25713015ce005be3b34b56377eaafa339be7c50c63ce7f3ce7841ae55f3221f
680f05ee98 build: fix version number post CMake (fanquake)
Pull request description:
CMake was merged after branching-off for `28.x`.
ACKs for top commit:
hebasto:
ACK 680f05ee98
TheCharlatan:
ACK 680f05ee98
vasild:
ACK 680f05ee98
Tree-SHA512: 18e10a49830b529f54f7c9ed81ff0489f9aac9779f34935ebe0051c742f3b7862ca5d3409a6d6347c2d9c6439dcdd1394333278308012fed473f5b00d80afdd5
41051290ab cmake: Ignore build subdirectories within source directory (Hennadii Stepanov)
6ce50fd9d0 doc: Update for CMake-based build system (Hennadii Stepanov)
9730288a0c ci: Migrate CI scripts to CMake (Hennadii Stepanov)
c360837ca5 cmake, lint: Adjust `lint_includes_build_config` (Hennadii Stepanov)
3885441ee0 cmake: Add presets for native Windows builds (Hennadii Stepanov)
7681746b20 cmake: Add vcpkg manifest file (Hennadii Stepanov)
8b6f1c4353 cmake: Add `Coverage` and `CoverageFuzz` scripts (Hennadii Stepanov)
65bdbc1ff2 cmake: Add `docs` build target (Hennadii Stepanov)
fb75ebbc33 cmake: Add compiler diagnostic flags (Hennadii Stepanov)
e821f0a37a cmake: Migrate Guix build scripts to CMake (Hennadii Stepanov)
747adb6ffe cmake: Add `Maintenance` module (Hennadii Stepanov)
1f60b30df0 cmake: Add `APPEND_{CPP,C,CXX,LD}FLAGS` cache variables (Hennadii Stepanov)
2b43c45b13 cmake: Add `AddWindowsResources` module (Hennadii Stepanov)
973a3b0c5d cmake: Implement `install` build target (Hennadii Stepanov)
84ac35cfd4 cmake: Add cross-compiling support (Hennadii Stepanov)
0d01c228a7 build: Generate `toolchain.cmake` in depends (Hennadii Stepanov)
91a799247d depends: Add host-specific `cmake_system_version` variables (Hennadii Stepanov)
9b31209b4c depends: Rename `cmake_system` -> `cmake_system_name` (Hennadii Stepanov)
4a5208a81d Revert "build, qt: Do not install *.prl files" (Hennadii Stepanov)
6522af62af depends: Amend handling flags environment variables (Hennadii Stepanov)
90cec4d251 cmake: Add `MULTIPROCESS` option (Hennadii Stepanov)
bb1a450dcb cmake: Build `bitcoin-chainstate` executable (Hennadii Stepanov)
aed38ea58c cmake: Build `bitcoinkernel` library (Hennadii Stepanov)
975d67369b cmake: Build `test_bitcoin-qt` executable (Hennadii Stepanov)
10fcc668a3 cmake: Add `WITH_DBUS` option (Hennadii Stepanov)
5bb5a4bc75 cmake: Add `libqrencode` optional package support (Hennadii Stepanov)
57a6e2ef4a cmake: Build `bitcoin-qt` executable (Hennadii Stepanov)
30f642952c cmake: Add `WERROR` option (Hennadii Stepanov)
c98d4a4c34 cmake: Add `REDUCE_EXPORTS` option (Hennadii Stepanov)
a01cb6e63f cmake: Add `HARDENING` option (Hennadii Stepanov)
a8a2e364ac cmake: Add Python-based tests (Hennadii Stepanov)
3d85379570 cmake: Add fuzzing options (Hennadii Stepanov)
908530e312 cmake: Add `SANITIZERS` option (Hennadii Stepanov)
8bb0e85631 cmake: Build `bench_bitcoin` executable (Hennadii Stepanov)
801735163a cmake: Add external signer support (Hennadii Stepanov)
353e0c9e96 cmake: Add `systemtap-sdt` optional package support (Hennadii Stepanov)
d2fda82b49 cmake: Add `libzmq` optional package support (Hennadii Stepanov)
ae7b39a0e1 cmake: Add `libminiupnpc` optional package support (Hennadii Stepanov)
6480e1dcdb cmake: Add `libnatpmp` optional package support (Hennadii Stepanov)
e73e9304a1 cmake: Build `bitcoin-util` executable (Hennadii Stepanov)
027c6d7caa cmake: Build `bitcoin-tx` executable (Hennadii Stepanov)
d10c5c34c3 cmake: Add wallet functionality (Hennadii Stepanov)
ab2e99b0d9 cmake: Create test suite for `ctest` (Hennadii Stepanov)
959370bd76 cmake: Build `test_bitcoin` executable (Hennadii Stepanov)
b27bf9700d cmake: Build `bitcoin-cli` executable (Hennadii Stepanov)
a9813df826 cmake: Build `bitcoind` executable (Hennadii Stepanov)
97829ce2d5 cmake: Add `FindLibevent` module (Hennadii Stepanov)
3118e40c61 cmake: Build `bitcoin_consensus` library (Hennadii Stepanov)
809a2f1929 cmake: Build `bitcoin_util` static library (Hennadii Stepanov)
0a9a521a70 cmake: Build `bitcoin_crypto` library (Hennadii Stepanov)
958971f476 cmake: Build `univalue` static library (Hennadii Stepanov)
752747fda8 cmake: Generate `obj/build.h` header (Hennadii Stepanov)
1f0a78edf3 cmake: Build `minisketch` static library (Hennadii Stepanov)
12bfbc8154 cmake: Build `leveldb` static library (Hennadii Stepanov)
51985c5304 cmake: Build `crc32c` static library (Hennadii Stepanov)
db7a198f29 cmake: Build `secp256k1` subtree (Hennadii Stepanov)
dbb7ed14e8 cmake: Add `ccache` support (Hennadii Stepanov)
cedfdf6c72 cmake: Redefine/adjust per-configuration flags (Hennadii Stepanov)
b6b5e732c8 cmake: Add global compiler and linker flags (Hennadii Stepanov)
f98327931b cmake: Add `TryAppendLinkerFlag` module (Hennadii Stepanov)
4a0af29697 cmake: Add `TryAppendCXXFlags` module (Hennadii Stepanov)
35cffc497d cmake: Add POSIX threads support (Hennadii Stepanov)
fd72d00ffe cmake: Add position independent code support (Hennadii Stepanov)
07069e2bb0 cmake: Add introspection module (Hennadii Stepanov)
27d687fc1f cmake: Add `config/bitcoin-config.h` support (Hennadii Stepanov)
fe5cdace5f cmake: Print compiler and linker flags in summary (Hennadii Stepanov)
70683884c5 cmake: Introduce interface libraries to encapsulate common flags (Hennadii Stepanov)
a2317e27b7 cmake: Add root `CMakeLists.txt` file (Hennadii Stepanov)
Pull request description:
This PR introduces a new CMake-based build system, which is a drop-in replacement for the current Autotools-based build system.
ML announcement: https://groups.google.com/g/bitcoindev/c/hgKkfQWzrTo
As discussed during the recent CoreDev meetup in April, the switch from Autotools to CMake is intended to happen as soon as possible after branching 28.x off, which means that 29.0 will be built using CMake.
This PR branch is essentially the [staging branch](https://github.com/hebasto/bitcoin/tree/cmake-staging), with every change reviewed and tested by a group of contributors, including (in alphabetical order):
- [**achow101**](https://github.com/achow101)
- [**fanquake**](https://github.com/fanquake)
- [**maflcko**](https://github.com/maflcko)
- [**m3dwards**](https://github.com/m3dwards)
- [**pablomartin4btc**](https://github.com/pablomartin4btc)
- [**real-or-random**](https://github.com/real-or-random)
- [**ryanofsky**](https://github.com/ryanofsky)
- [**sipsorcery**](https://github.com/sipsorcery)
- [**TheCharlatan**](https://github.com/TheCharlatan)
- [**theStack**](https://github.com/theStack)
- [**theuni**](https://github.com/theuni)
- [**vasild**](https://github.com/vasild)
Reviewing in a separate staging repo was suggested in https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1431798320.
The accompanying changes to the OSS-Fuzz project are available in https://github.com/hebasto/oss-fuzz/pull/8.
Please refer to the [build options parity table](https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e). The "auto" value is no longer available; non-default values must be specified explicitly. Additionally, the new default values have been chosen to suit the everyday build experience for the majority of developers.
System requirements for using the CMake-based build system:
- CMake >= 3.22 (if not available in your system's repository, it can be downloaded from https://cmake.org/download/)
- a build tool of your choice:
- any Make (GNU Make is no longer a requirement); GNU Make is still required to build depends
- Ninja (https://ninja-build.org/)
- MSBuild
- Xcode
A note for Windows users: The default installation of the latest version of MSVC 17.10.4 includes both CMake 3.28.3 and the vcpkg package manager).
---
We, the build system developers, kindly ask reviewers to refrain from making suggestions that are not directly related to the migration process or can be implemented separately. Bugs in the scripts and errors in the updated documentation should be the focus of this PR. Please be advised that comments not aligned with this PR's goal may be ignored.
Thank you all for your understanding.
ACKs for top commit:
maflcko:
review ACK 41051290ab🐥
sipsorcery:
ACK 41051290ab.
vasild:
ACK 41051290ab
TheCharlatan:
ACK 41051290ab
pablomartin4btc:
tACK 41051290ab
i-am-yuvi:
tACK [`4105129`](41051290ab)
theuni:
ACK 41051290ab.
fanquake:
ACK 41051290ab
Tree-SHA512: 6c1445054436c6c00ad63bfa0f19d64091a2b25c9bd694f85bf2218ac358ffb774d6c000685b3ca1e9b50401babed989fa2a0694b774c211d226bfd1944c9b39
fad0cf6f26 refactor: Use std::ranges::equal in GetNetworkForMagic (MarcoFalke)
fadf0a7e15 refactor: Remove Span operator==, Use std::ranges::equal (MarcoFalke)
Pull request description:
`std::span` removed the comparison operators, so it makes sense to remove them for the `Span` "backport" as well. Using `std::ranges::equal` also has the benefit that some `Span` temporary constructions can now be dropped.
This is required to move from `Span` toward `std::span`.
ACKs for top commit:
hodlinator:
Untested Code Review re-ACK fad0cf6
stickies-v:
ACK fad0cf6f26
TheCharlatan:
ACK fad0cf6f26
Tree-SHA512: 5b9d1826ceac2aabae2295bc89893dd23ac3a1cc0d41988331cdbdc21be531aa91795d5273819f349f79648c6c4f30ed31af6e7a3816153e92080061b92ffe00
8dec4e1294 fuzz: fix timeout in `crypto_fschacha20poly1305` (brunoerg)
Pull request description:
Fixes#30505
This PR fixes a timeout in `crypto_fschacha20poly1305` by reducing the number of iterations. I left it running for a while and noticed it speeds up the target and do not impact coverage.
ACKs for top commit:
maflcko:
lgtm ACK 8dec4e1294
stratospher:
ACK 8dec4e1. saw similar coverage stats (these are from different machines, saw more similar from same machine).
Tree-SHA512: 66cd689f10f09cf880c1f2c77aa481336a95a7e10b655d156b2dc9cd38165670000019a6fb3775485ccb5cce248939a52a31ebd755a6befed6274cc841d21d97
49f9b645ea refactor: Testnet4 - Replace uint256S("str") -> uint256{"str"} (Hodlinator)
Pull request description:
Ran scripted-diff from 2d9d752e4f:
```
sed -i --regexp-extended -e 's/\buint256S\("(0x)?([^"]{64})"\)/uint256{"\2"}/g' $(git grep -l uint256S)
```
Follow-up to Testnet4 introduction #29775 which overlapped with work on `uint256` `consteval` ctor #30560 (the latter includes the scripted-diff commit).
Going forward `uint256{}` should be used for constants instead of `uint256S()`.
ACKs for top commit:
maflcko:
review-ACK 49f9b645ea🐮
fjahr:
ACK 49f9b645ea
Tree-SHA512: 94fe5d9f1fb85e9ce5c3c4c5e4c31667e8cbb55ee691a4b5b3ae4172ccac38230281071023663965917f188b4c19bdf67afd4e3cdf69d89e97c65faea88af833
18d65d2772 test: use uint256::FromUserHex for RANDOM_CTX_SEED (stickies-v)
6819e5a329 node: use uint256::FromUserHex for -assumevalid parsing (stickies-v)
2e58fdb544 util: remove unused IsHexNumber (stickies-v)
8a44d7d3c1 node: use uint256::FromUserHex for -minimumchainwork parsing (stickies-v)
70e2c87737 refactor: add uint256::FromUserHex helper (stickies-v)
85b7cbfcbe test: unittest chainstatemanager_args (stickies-v)
Pull request description:
Since fad2991ba0, `uint256S` has been [deprecated](fad2991ba0 (diff-800776e2dda39116e889839f69409571a5d397de048a141da7e4003bc099e3e2R138)) because it is less robust than the `base_blob::FromHex()` introduced in [the same PR](https://github.com/bitcoin/bitcoin/pull/30482). Specifically, it tries to recover from length-mismatches, recover from untrimmed whitespace, 0x-prefix and garbage at the end, instead of simply requiring exactly 64 hex-only characters. _(see also #30532)_
This PR carves out the few `uint256S` callsites that may potentially prove a bit more controversial to change because they deal with user input and backwards incompatible behaviour change.
The main behaviour change introduced in this PR is:
- `-minimumchainwork` will raise an error when input is longer than 64 hex digits
- `-assumevalid` will raise an error when input contains invalid hex characters, or when it is longer than 64 hex digits
- test: the optional RANDOM_CTX_SEED env var will now cause tests to abort when it contains invalid hex characters, or when it is longer than 64 hex digits
After this PR, the remaining work to remove `uint256S` completely is almost entirely mechanical and/or test related. I will open that PR once #30560 is merged because it builds on that.
ACKs for top commit:
hodlinator:
re-ACK 18d65d2772
l0rinc:
ACK 18d65d2772
achow101:
ACK 18d65d2772
ryanofsky:
Code review ACK 18d65d2772. Very nice change that cleans up the API, adds checking for invalid values, makes parsing of values more consistent, and adds test coverage.
Tree-SHA512: ec118ea3d56e1dfbc4c79acdbfc797f65c4d2107b0ca9577c848b4ab9b7cb8d05db9f3c7fe8441a09291aca41bf671572431f4eddc59be8fb53abbea76bbbf86
fa7b9b99a2 refactor: Require std::input_iterator for all InputIterator in prevector (MarcoFalke)
d444441900 refactor: Allow CScript construction from any std::input_iterator (MarcoFalke)
Pull request description:
Currently only (pre)vector iterators and raw pointers are accepted. However, this makes it harder to construct from input iterators provided by other classes, such as `std::span`.
Fix that.
ACKs for top commit:
delta1:
reACK fa7b9b9
achow101:
ACK fa7b9b99a2
hodlinator:
ACK fa7b9b99a2
ryanofsky:
Code review ACK fa7b9b99a2
Tree-SHA512: 2760861f8bce42fb27dc0825e61621cb157f1ac3619a0834df38eb8319b6dcf9de43d90397a4c160f43340880c1553df638848e9057a27c792214331243ef4a5
fab0e834b8 bench: [refactor] iwyu (MarcoFalke)
Pull request description:
Missing includes are problematic, because:
* Upcoming releases of a C++ standard library implementation often minimize their internal header dependencies. For example, `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` (https://libcxx.llvm.org/DesignDocs/HeaderRemovalPolicy.html). This can lead to compile failures, which are easy to fix for developers, but may not be for users. For example, commit 138f867156 had to add missing includes to accommodate GCC 15 (and the commit had to be backported).
* A Bitcoin Core developer removing a feature from a module and wanting to drop the now unused includes may not be able to do so without touching other unrelated files, because those files rely on the transitive includes.
Moreover, missing or extraneous includes are problematic, because they may be confusing the code reader as to what the real dependencies are.
Finally, extraneous includes may slow down the build.
Fix all issues in `bench`, by applying the rule include-what-you-use (iwyu).
Follow-up pull requests will handle the other places.
ACKs for top commit:
hodlinator:
ACK fab0e834b8
achow101:
ACK fab0e834b8
TheCharlatan:
ACK fab0e834b8
hebasto:
ACK fab0e834b8.
brunoerg:
crACK fab0e834b8
stickies-v:
ACK fab0e834b8
Tree-SHA512: f079c05d3ddebafabbd5a6c76d43d17337d1a962b97ba0ee27612f91c58491e7ce4e4229be54bd6e75a15512798c6f59925d0a076a37c050f8b9ef455ae5c9a2
1bf9b70658 docs: Add 379 and 387 to bips.md (Ava Chow)
35ef34eab7 docs: Remove release 28.0 release notes fragments (Ava Chow)
f5cf43bb91 build: Bump to 28.99 (Ava Chow)
Pull request description:
* Bump to 28.99 in preparation for the 28.x branching
* Remove current release note fragments. They've been moved to [draft release notes]((https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft).) in the wiki.
* Updated bips.md with missing BIPs that were implemented a while ago.
ACKs for top commit:
maflcko:
lgtm ACK 1bf9b70658
tdb3:
re ACK 1bf9b70658
Tree-SHA512: 9d5b05330268bb233369d78f06011dbdcdcb87770c499ae743ac4833735ba657b6611ff1e332c9d4added854b694c5a3a961d911247303f7ddaaaa72149d5574
b061b35105 seeds: Regenerate mainnet seeds (virtu)
02dc45c506 seeds: Pull nodes from Luke's seeder (virtu)
7a2068a0ff seeds: Pull nodes from virtu's crawler (virtu)
Pull request description:
This builds on #30008 and adds data [exported](https://github.com/virtu/seed-exporter) by [my crawler](https://github.com/virtu/p2p-crawler) an additional source for seed nodes. Data covers all supported network types.
[edit: Added Luke's seeder as input as well.]
### Motivation
- Further decentralizes the seed node selection process (in the long term potentially enabling an _n_-source threshold for nodes to prevent a single source from entering malicious nodes)
- No longer need to manually curate seed node list for any network type: See last paragraph of OP in #30008. My crawler has been [discovering the handful of available cjdns nodes](https://21.ninja/reachable-nodes/nodes-by-net-type/) for around two months, all but one of which meet the reliability criteria.
- Alignment of uptime requirements for Onion and I2P nodes with those of clearnet nodes to 50%: If I'm reading the code correctly, seeders appear to optimize for up-to-dateness by using [lower connection timeouts](3c1a63c672/src/crawl.rs (L349)) than [Bitcoin Core](bc87ad9854/src/netbase.cpp (L40C27-L40C48)) to maximize throughput. Since my crawler does not have the same timeliness requirements, it opts for accuracy by using generous timeouts. As a result, its data contains additional eligible Onion (and other darknet nodes), as is shown in the histogram below. Around 4500 Onion nodes are discovered so far (blue); my data adds ~6400 more (orange); ~ 1500 nodes take longer than the default 20-second Bitcoin Core timeout and won't qualify as "good".
Here's the current results with 512 nodes for all networks except cjdns:
<details>
<summary>Using the extra data</summary>
```
IPv4 IPv6 Onion I2P CJDNS Pass
10335 2531 11545 1589 10 Initial
10335 2531 11545 1589 10 Skip entries with invalid address
5639 1431 11163 1589 8 After removing duplicates
5606 1417 11163 1589 8 Enforce minimal number of blocks
5606 1417 11163 1589 8 Require service bit 1
4873 1228 11163 1589 8 Require minimum uptime
4846 1225 11161 1588 8 Require a known and recent user agent
4846 1225 11161 1588 8 Filter out hosts with multiple bitcoin ports
512 512 512 512 8 Look up ASNs and limit results per ASN and per net
```
</details>
<details>
<summary>Before</summary>
```
IPv4 IPv6 Onion I2P CJDNS Pass
5772 1323 443 0 2 Initial
5772 1323 443 0 2 Skip entries with invalid address
4758 1110 443 0 2 After removing duplicates
4723 1094 443 0 2 Enforce minimal number of blocks
4723 1094 443 0 2 Require service bit 1
3732 867 443 0 2 Require minimum uptime
3718 864 443 0 2 Require a known and recent user agent
3718 864 443 0 2 Filter out hosts with multiple bitcoin ports
512 409 443 0 2 Look up ASNs and limit results per ASN and per net
```
</details>
### To dos
- [x] Remove manual nodes and update README
- [x] Mark nodes with connection times exceeding Bitcoin Core's default as bad in [exporter](https://github.com/virtu/seed-exporter): [done](https://github.com/virtu/seed-exporter/pull/12)
- [x] Regenerate mainnet seeds
- [x] Rebase, then remove WIP label once #30008 gets merged
ACKs for top commit:
achow101:
ACK b061b35105
fjahr:
utACK b061b35105
Tree-SHA512: 63e86220787251c7e8d2d5957bad69352e19ae17d7b9b2d27d8acddfec5bdafe588edb68d77d19c57f25f149de723e2eeadded0c8cf13eaca22dc33bd8cf92a0
1b41d45d46 wallet: bugfix: ensure atomicity in settings updates (ismaelsadeeq)
Pull request description:
This PR fixes#30620.
As outlined in the issue, creating two wallets with `load_on_startup=true` simultaneously results in only one wallet being added to the startup file.
The current issue arises because the wallet settings update process involves:
1. Obtaining the settings value while acquiring the settings lock.
2. Modifying the settings value.
3. Overwriting the settings value while acquiring the settings lock again.
This sequence is not thread-safe. Different threads could modify the same base value simultaneously, overwriting data from other workers without realizing it.
The PR attempts to fix this by modifying the chain interface's `updateRwSetting` method to accept a function that will be called with the settings reference. This function will either update or delete the setting and return an enum indicating whether the settings need to be overwritten in this or not.
Additionally, this PR introduces two new methods to the chain interface:
- `overwriteRwSetting`: This method replaces the setting with a new value.
Used in `VerifyWallets`
- `deleteRwSettings`: This method completely erases a specified setting.
This method is currently used only in `overwriteRwSetting`.
These changes ensure that updates are race-free across all clients.
ACKs for top commit:
achow101:
ACK 1b41d45d46
furszy:
self-code-ACK 1b41d45d46
Tree-SHA512: 50cda612b782aeb5e03e2cf63cc44779a013de1c535b883b57af4de22f24b0de80b4edecbcda235413baec0a12bdf0e5750fb6731c9e67d32e742d8c63f08c13
fa5b58ea01 test: Avoid intermittent block download timeout in p2p_ibd_stalling (MarcoFalke)
Pull request description:
Fixes#30704
The goal of the test is to check the stalling timeout, not the block download timeout.
On extremely slow hardware (for example qemu virtual hardware), downloading the 1023 blocks may take longer than the block download timeout.
Fix it by pinning the time using mocktime, and only advance it when testing the stalling timeout.
ACKs for top commit:
tdb3:
CR ACK fa5b58ea01
brunoerg:
utACK fa5b58ea01
Tree-SHA512: 9a9221f264bea52be5e9fe81fd319f5a6970cd315cc5e9f5e2e049c5d84619b19b9f6f075cda8d34565c2d6c17a88fb57e195c66c271e40f73119a77caecb6d7
