In test/functional/interface_usdt_net.py, assert_equal is already
used to check for equality between objects. Replace 'assert.*=='
with 'assert_equal' and 'assert.*>' with 'assert_greater_than'
to further easify debugging.
21e9d39a3725cd6107b742f0cb97f65b3640201b docs: add release notes for 31603 (brunoerg)
a8b548d75d9a376c9bb66e06bb918c876416d615 test: `getdescriptorinfo`/`importdescriptors` with whitespace in pubkeys (brunoerg)
c7afca3d62cf5d3ea9b98d5a76e4e54cac07bc3c test: descriptor: check whitespace into keys (brunoerg)
cb722a3cea16a04844c83e56fd6deaa1f0dc0a7e descriptor: check whitespace in ParsePubkeyInner (brunoerg)
50856695ef6c02ecbaa0cf448567355b6b86b510 test: fix descriptors in `ismine_tests` (brunoerg)
Pull request description:
Currently, we successfully parse descriptors which contains spaces in the beginning or end of the public/private key within a fragment (e.g. `pk( KEY)`, `pk(KEY )` or `pk( KEY )`). I have noticed that one of the reasons is that the `DecodeBase58` function simply ignore these whitespaces.
This PR changes the `ParsePubkeyInner ` to reject pubkeys that contain a whitespace at the beginning and/or at the end. We will only check the whitespace in some RPCs (e.g. `importdescriptors`), but an already imported descriptor won't be affected by this check, especially because we store descriptors from `ToString`.
For context: https://github.com/brunoerg/bitcoinfuzz/issues/72
ACKs for top commit:
rkrux:
tACK 21e9d39a3725cd6107b742f0cb97f65b3640201b
darosior:
re-ACK 21e9d39a3725cd6107b742f0cb97f65b3640201b
sipa:
utACK 21e9d39a3725cd6107b742f0cb97f65b3640201b
Tree-SHA512: 54f48a89a235517e5cdc29a46dceeb7dabbee93c7616a166288ff3f90131808eb0ece43b0797a11fe827a5f7bd51d65e3e75c16789b0a42020934cabb684cc8f
Pass bitcoin binary command lines from test framework to signet/miner utility
using shell escaping so they are unambigous and don't get mangled if they
contain spaces.
This change is not needed for tests to pass currently, but is a useful change
to avoid CI failures in followup PR
https://github.com/bitcoin/bitcoin/pull/31375 and to avoid other bugs.
Add new TestNode.binaries object to manage paths to bitcoin binaries.
Having this object makes it possible for the test framework to exercise the
bitcoin wrapper executable introduced in
https://github.com/bitcoin/bitcoin/pull/31375 and also makes it easier to add
new binaries and options and environment variables controlling how they are
invoked, because logic for invoking them that was previously spread out is now
consolidated in one place.
Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>
1f9b2e150ce5aa192226d2daae73f7d678c47d65 cmake: Require `zip` only for `deploy` target (Hennadii Stepanov)
0aeff2995138c247f1b1f843c30eb547e199f5f7 cmake: Check for `makensis` tool before using it (Hennadii Stepanov)
Pull request description:
For `x86_64-w64-mingw32` and `*-apple-darwin` targets, the optional `deploy` target requires dedicated tools: `makensis` and `zip`, respectively.
This PR introduces a uniform checks for those tools when attempting to build the `deploy` target, ensuring they are not required for configuring and building any other targets.
Here is an example of workflow for `x86_64-w64-mingw32`:
```
$ # `nsis` is not installed
$ cmake -B build -G "GNU Makefiles" --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
$ cmake --build build -j $(nproc)
$ cmake --build build -t deploy
Error: NSIS not found.
Please install NSIS and/or ensure that its executable is accessible to the find_program() command—
for example, by setting the MAKENSIS_EXECUTABLE variable or another relevant CMake variable.
Then re-run cmake to regenerate the build system.
Built target deploy
$ sudo apt install nsis
$ cmake -B build
$ cmake --build build -t deploy
...
[100%] Generating bitcoin-win64-setup.exe
[100%] Built target deploy
```
Fixes https://github.com/bitcoin/bitcoin/issues/32018.
ACKs for top commit:
hodlinator:
re-ACK 1f9b2e150ce5aa192226d2daae73f7d678c47d65
fanquake:
ACK 1f9b2e150ce5aa192226d2daae73f7d678c47d65
Tree-SHA512: 5e2bd28a13bd8fa7c4ba8cf1756d200a4651afe83c463d76ece10027cca343e124eff97012a5368028f761df60f420ab891106b4e33b50045051d57c7464ff98
This reverts the changes to test/functional/wallet_crosschain.py
in commit 74a04f9e7ad6a16988149cc3438b9ce13c91cdb9.
It's sufficient to check only one test network. The next commit
will change that network away from testnet3.
54e6eacc1fccd602897d9e3025c62f83194ffd5b test: Enable ResetCoverageCounters beyond Linux (janb84)
Pull request description:
In PR [#31901](https://github.com/bitcoin/bitcoin/pull/31901), Coverage.cpp was introduced as a separate utility file, based on existing code. However, the macro defined in Coverage.cpp was limited to Clang and Linux, which caused issues for users on macOS when using the newly introduced deterministic test tooling.
This change adds fallback functions which are used when building without code coverage on non linux env.
This adds support for macOS to ResetCoverageCounters. ResetCoverageCounters is used by the unit tests in `g_rng_temp_path_init` to support the deterministic unit test tooling. It is also used in fuzz tests to completely suppress coverage from anything init-related.
See [Readme](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/README.md) on how to test this for deterministic unit & fuzz test.
Suggestion for test files:
- for unit test: `util_string_tests`
- for fuzz test: `addition_overflow `
These files should give deterministic results
ACKs for top commit:
maflcko:
review-only ACK 54e6eacc1fccd602897d9e3025c62f83194ffd5b
hodlinator:
re-ACK 54e6eacc1fccd602897d9e3025c62f83194ffd5b
Tree-SHA512: dd71da6f76d4fc9e64bf521bbfe5e7483d77c2ca0380f9e692502e64b529068ea33f21b19399481feb7c6780a23d893d8b7f733cef641a2db18a13397c98deea
7d34c19853e7a5528d69c5f30580e7e9712e61f0 ci: Drop ENABLE_HARDENING=OFF from clang-tidy (David Gumberg)
Pull request description:
Split out from #32071
It's not clear why this was added in the first place, but it is not necessary currently.
https://github.com/bitcoin/bitcoin/pull/32038#issuecomment-2723888193https://github.com/bitcoin/bitcoin/pull/24753.
ACKs for top commit:
fanquake:
ACK 7d34c19853e7a5528d69c5f30580e7e9712e61f0
Tree-SHA512: 9ffab10349f52aed77d002155da59f13f0954e6bc1ac1edbddcbb9e70aa2db3b0b0307eb35f8b791589f2157705a96d463589d4270f4873e9037a9ef88807b56
fa3b4427158d48f7d899582580f8f6a7b1bc981d ci: Use Cirrus dockerfile cache (MarcoFalke)
Pull request description:
The lint task is problematic, because:
* It doesn't check modifications to `ci/lint_imagefile`
* It calls a separate script that installs packages on every run (taking time)
* It uses `*_cache` instructions to cache some installed parts, but not all
Fix all issues by using `ci/lint_imagefile` (https://cirrus-ci.org/guide/docker-builder-vm/#dockerfile-as-a-ci-environment)
ACKs for top commit:
willcl-ark:
ACK fa3b4427158d48f7d899582580f8f6a7b1bc981d
Tree-SHA512: 243d78219639b83721d4e5cb32d16e5c208a61c919d04646279be5825ba92d97c490b5d4d28ea103eb820b1a259904574cb3e32eaca3f11c3031810e3d87ff4a
6f9f415a4fa3a3c1dc87c207b0192a5ed0a91e0f doc: shallow clone qa-assets (Lőrinc)
Pull request description:
While reviewing https://github.com/bitcoin/bitcoin/pull/31457#pullrequestreview-2690077410 I noticed that cloning `qa-assets` takes a lot of time - shallow cloning should suffice here.
I haven't checked the other clones in this file but suggestion are welcome.
ACKs for top commit:
maflcko:
lgtm ACK 6f9f415a4fa3a3c1dc87c207b0192a5ed0a91e0f
Tree-SHA512: 21bd676c7709dbf7fd30b239d0a72f9c230453ed8f8a1b5319ac92ef9c5e67780939f095a239dd31bcb4550f8d69eaed4931a221e19cb0b957f18fac623c4a01
We might have just set CNode::fDisconnect in the first loop because of being offline.
Also caches CConnman::fNetworkActive in case it's changed concurrently with our own thread.
52482cb24400f8c44ba9628aaaecb7c04b11beb2 test: Check datadir cleanup after assumeutxo was successful (Fabian Jahr)
Pull request description:
I noticed that the proper datadir cleanup after a successful restart of an assumutxo node does not seem to be covered in our tests. This is added here.
ACKs for top commit:
l0rinc:
utACK 52482cb24400f8c44ba9628aaaecb7c04b11beb2
mabu44:
Re-ACK 52482cb24400f8c44ba9628aaaecb7c04b11beb2
Prabhat1308:
re-ACK [`52482cb`](52482cb244)
TheCharlatan:
Re-ACK 52482cb24400f8c44ba9628aaaecb7c04b11beb2
Tree-SHA512: cc941afeba250050eaccf5112255d961253fec9b2683545454a0d2fbe4d542178394b301d169a9dd79edbf6b5d478d95282727dbb0aca96ee79d4cd1ff80f19b
7ebc458a8cb994bc3c0c129da61353968d955bc2 qt: doc: adapt outdated binary paths to CMake changes (Sebastian Falbesoner)
Pull request description:
Adapt the qt-related instances of outdated binary paths to `./build/bin/...` (see [#30454](https://github.com/bitcoin/bitcoin/pull/30454) and the more recently merged [#31161](https://github.com/bitcoin/bitcoin/pull/31161)). According to `$ git grep src/qt.*bitcoin` there should be no more left to address.
ACKs for top commit:
maflcko:
lgtm ACK 7ebc458a8cb994bc3c0c129da61353968d955bc2
Sjors:
utACK 7ebc458a8cb994bc3c0c129da61353968d955bc2
fanquake:
ACK 7ebc458a8cb994bc3c0c129da61353968d955bc2
hebasto:
ACK 7ebc458a8cb994bc3c0c129da61353968d955bc2.
Tree-SHA512: 8cd6579fdf209ec4ee3c4c9cfb94cb11d5d5115068d31613d356ca1303214dc4461580535c2d3f2773f373a4271e9a82df25596d8369eef8235822f7030d88bd
4cd95a2921805f447a8bcecc6b448a365171eb93 refactor: modernize remaining outdated trait patterns (Lőrinc)
ab2b67fce20fd7d8017f8a26425cab99e91f420d scripted-diff: modernize outdated trait patterns - values (Lőrinc)
8327889f358289f918d04ddb9469fb5562720bf4 scripted-diff: modernize outdated trait patterns - types (Lőrinc)
Pull request description:
The use of [`std::underlying_type_t<T>`](https://en.cppreference.com/w/cpp/types/underlying_type) or [`std::is_enum_v<T>`](https://en.cppreference.com/w/cpp/types/is_enum) (and similar ones, introduced in C++14) replace the `typename std::underlying_type<T>::type` and `std::is_enum<T>::value` constructs (available in C++11).
The `_t` and `_v` helper alias templates offer a more concise way to extract the type and value directly.
I've modified the instances I found in the codebase one-by-one (noticed them while investigating https://github.com/bitcoin/bitcoin/pull/31868), and afterwards extracted scripted diff commits to do the trivial ones automatically.
The last commit contains the values that were easier done manually.
I've excluded changes from `src/bench/nanobench.h`, `src/leveldb`, `src/minisketch`, `src/span.h` and `src/sync.h` - let me know if you think they should be included instead.
A few of the code changes can also be reproduced by clang-tidy (but not all of them):
```bash
cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_FOR_FUZZING=ON && cmake --build build -j$(nproc)
run-clang-tidy -quiet -p build -j $(nproc) -checks='-*,modernize-type-traits' -fix $(git grep -lE '::(value|type)' ./src ':(exclude)src/bench/nanobench.h' ':(exclude)src/leveldb' ':(exclude)src/minisketch' ':(exclude)src/span.h' ':(exclude)src/sync.h')
```
ACKs for top commit:
laanwj:
Concept and code review ACK 4cd95a2921805f447a8bcecc6b448a365171eb93
Tree-SHA512: a4bcf0f267c0f4e02983b4d548ed6f58d464ec379ac5cd1f998b9ec0cf698b53a9f2557a05a342b661f1d94adefc9a0ce2dc8f764d49453aaea95451e2c4c581
3301d2cbe8c3b76c97285d75fa59637cb6952d0b qa: Wait for txindex to avoid race condition (Hodlinator)
9bfb0d75ba10591cc6c9620f9fd1ecc0e55e7a48 qa: Remove unnecessary -txindex args (Hodlinator)
7ac281c19cd3d11f316dbbb3308eabf1ad4f26d6 qa: Add missing coverage of corrupt indexes (Hodlinator)
Pull request description:
- Add synchronization in 3 places where if the Transaction Index happens to be slow, we get rare test failures when querying it for transactions (one such case experienced on Windows, prompting investigation).
- Remove unnecessary TxIndex initialization in some tests.
- Add some test coverage where TxIndex aspect could be tested in feature_init.py.
ACKs for top commit:
fjahr:
re-ACK 3301d2cbe8c3b76c97285d75fa59637cb6952d0b
mzumsande:
Code Review ACK 3301d2cbe8c3b76c97285d75fa59637cb6952d0b
furszy:
Code review ACK 3301d2cbe8c3b76c97285d75fa59637cb6952d0b
Prabhat1308:
Concept ACK [`3301d2c`](3301d2cbe8)
Tree-SHA512: 7c2019e38455f344856aaf6b381faafbd88d53dc88d13309deb718c1dcfbee4ccca7c7f1b66917395503a6f94c3b216a007ad432cc8b93d0309db9805f38d602
36b0713edc4655f6e0c291975d6d280fbc89cf2e test: fix intermittent failure in wallet_reorgsrestore.py (furszy)
Pull request description:
In response to #32066 intermittent failure.
Wait until the node's process has fully stopped before starting a new instance of it.
Same behavior as in the [tool_wallet.py](698f86964c/test/functional/tool_wallet.py (L540)) test.
ACKs for top commit:
maflcko:
lgtm ACK 36b0713edc4655f6e0c291975d6d280fbc89cf2e
Chand-ra:
tACK [36b0713](36b0713edc)
Tree-SHA512: 8e01493ef1fb58589479f3e12d7429d02ca75a2183d5f79d3b6a2fbf13334878926274a20857f1b4729afc1d30b65789daed229ce06ba236b91d949b73f45d5a
02942056fd861581503a8a35a06dcf22d4ba1473 test: fix intermittent failure in p2p_orphan_handling.py (Martin Zumsande)
Pull request description:
If the mocktime is bumped before the node has successfully disconnected the peer, the requests for both parents could be spread over two GETDATAS: The first time `GetRequestsToSend` is invoked it would only request one tx from peer2, because the other one would only be available after peer1 was disconnected and its outstanding txrequest cleared.
So two GETDATAs would be sent, which would make the test fail.
Fixes#31700
ACKs for top commit:
maflcko:
lgtm ACK 02942056fd861581503a8a35a06dcf22d4ba1473
instagibbs:
ACK 02942056fd
Tree-SHA512: 769200898345da197d86d673d9506f08f0a64b72a456e7e7c988ac37450d9c54ec65da1c8447c566c8578f7cfccdc5723ea680e636bfbe0b3d38265e5ef57774
Non-Linux linkers require a fallback implementation for when coverage is not enabled.
The fallbacks are marked weak to have lower precedence than built-in implementations when available, removing ambiguity from the linker.
9157d9e449870851ef455e077249ac46fc2df24c build: use make < 3.82 syntax for define directive (Sjors Provoost)
Pull request description:
From the GNU make 3.82 [release announcement](https://lists.gnu.org/archive/html/info-gnu/2010-07/msg00023.html) (2010):
> The 'define' make directive now allows a variable assignment operator
after the variable name, to allow for simple, conditional, or appending
multi-line variable assignment.
macOS ships with 3.81 (2006). This caused the multiprocess config options to be ignored.
Fixes#32068
ACKs for top commit:
ryanofsky:
Code review ACK 9157d9e449870851ef455e077249ac46fc2df24c. This is a pretty unusual bug and I don't understand how it wasn't causing any errors with make 3.81, just causing the flags to be ignored.
Tree-SHA512: a07322e25ef18296264379a2704f31c654df196d3ea09fe712885c38813e54d758a2d603ee9f7a302da8011fba6d139aa30a356175ca99df728ade2572a87560
fa9cf38ab666b50b0d8e82cb17f9e5a8a613547d scripted-diff: test: Rename send_message to send_without_ping (MarcoFalke)
fa4356717d6efcc41709f62a5116c62519d1a853 test: Prefer send_and_ping over send_message+sync_with_ping (MarcoFalke)
Pull request description:
`send_message` is problematic, because it is easy to forget a `sync_with_ping` (or other `wait_until`), leading to intermittent test failures. (Example: https://github.com/bitcoin/bitcoin/pull/31837#discussion_r1950370246)
There are more uses of `send_and_ping` in the codebase than `send_message`, so in most cases `send_and_ping` is needed anyway.
For the remaining cases, clearly document that no sync happens by renaming `send_message` to `send_without_ping`.
ACKs for top commit:
instagibbs:
ACK fa9cf38ab6
Tree-SHA512: 31caa6568d292ae3d3dda931a94aaa30cc1205ec2ef537a484393eb55687f86c212f1e751ac4a7636610bdf591502a50995dc63bf02f97be9fdc482072160b07
d5537c18a9034647ba4c9ed4008abd7fee33989e fuzz: make sure DecodeBase58(Check) is called with valid values more often (Lőrinc)
bad1433ef2b5b02ac4b1c6c1d9482c513e5b2192 fuzz: Always restrict base conversion input lengths (Lőrinc)
Pull request description:
This is a follow-up to https://github.com/bitcoin/bitcoin/pull/30746, expanding coverage by:
* restricting every input for the base58 conversions, capping max sizes to `100` instead of `1000` or all available input (suggested by marcofleon in https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1963718683) since most actual usage has lengths of e.g. `21`, `34`, `78`.
* providing more valid values to the decoder (suggested by maflcko in https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1957847712) by randomly providing a random input or a valid encoded one; this also enables unifying the roundtrip tests to a single roundtrip per fuzz.
ACKs for top commit:
mzumsande:
Code Review / lightly tested ACK d5537c18a9034647ba4c9ed4008abd7fee33989e
maflcko:
review ACK d5537c18a9034647ba4c9ed4008abd7fee33989e 🚛
Tree-SHA512: 50365654cdac8a38708a7475eaa43396642b7337e2ee8999374c3faafff4f05457abc1a54c701211e0ed24d36c12af77bcad17b49695699be42664f2be660659
2819c514825577dc8d73690882f25117d412ed9f test: Use rpc_deprecated only for testing deprecation (Fabian Jahr)
Pull request description:
The comment in `functional/rpc_deprecated.py` says "This test should be used to verify correct behaviour of deprecated RPC methods with and without the -deprecatedrpc flags." I think we can get rid of the "with" part since we can assume that every deprecated RPC is already tested in at least one other functional test. (I didn't look but I could verify in our coverage if someone has doubts about that.) In order for this test to continue working, the flag will need to be used there. Otherwise this seems to prescribe copy+pasting a basic test from another file and I don't see a good reason for that.
ACKs for top commit:
maflcko:
lgtm ACK 2819c514825577dc8d73690882f25117d412ed9f
janb84:
re ACK [2819c51](2819c51482)
polespinasa:
reACK 2819c514825577dc8d73690882f25117d412ed9f
Tree-SHA512: 96edfd07be863ad19f99feb27afbc2c3ad53560c93c93eac8de8d766ad8c46e5aa02fd013fec99af794cbe9adca8e459d5b80b454caea7e67b2388003e010bb6
feature_config_args.py incorrectly assumed that its testnet4 node
would not log a disk space warning.
0683b8ebf33386d5c05140df89df10b1853d7c7e increased m_assumed_blockchain_size
on testnet4 from 1 to 11 GiB which triggers this bug on more
systems, e.g. a RAM disk.
Prevent the warning by setting -prune for these nodes.
Fix the same issue in feature_signet.py
From the GNU make 3.82 release announcement:
* The 'define' make directive now allows a variable assignment operator
after the variable name, to allow for simple, conditional, or appending
multi-line variable assignment.
macOS ships with 3.81. This caused the multiprocess config options
to be ignored.
Fixes#32068
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Wait until the node's process has fully stopped before starting a new instance.
Since the same code is used in tool_wallet.py, this consolidates the behavior
into a 'kill_process()' function.
send_message only drops the bytes in a buffer and a sync is needed to
avoid intermittent test issues. Change the name of the method to make
this more apparent during review.
-BEGIN VERIFY SCRIPT-
sed -i 's/send_message(/send_without_ping(/g' $( git grep -l 'send_message(' )
-END VERIFY SCRIPT-
36b6f36ac4724cb2c9ed0e25314c3bbf55e4ebb8 build: require sqlite when building the wallet (Sjors Provoost)
Pull request description:
Require that sqlite is available in order to compile the wallet. Removes instances of `USE_SQLITE` since it is no longer possible to not have sqlite available.
The `NO_SQLITE` option is dropped from depends.
This is another step towards dropping the legacy wallet, extracted from #31250.
ACKs for top commit:
m3dwards:
ACK 36b6f36ac4724cb2c9ed0e25314c3bbf55e4ebb8
davidgumberg:
crACK 36b6f36ac4
hebasto:
re-ACK 36b6f36ac4724cb2c9ed0e25314c3bbf55e4ebb8.
Tree-SHA512: 870a0135671c80c4f28602119eb8637a1ed43b51b1673bfe88425782fb62ec6ef0f3d6baf0d5984d6a243779b0f63423fd4c4dc324ef87bffba13d63e05ad793
80b5e7f2cb7fbfbd724e1f52b00c0e72b79a200b build: Remove manpages when making MacOS app (Ava Chow)
Pull request description:
When creating the MacOS app, the only file that should be in `Bitcoin-Qt.app/Contents/MacOS` is `Bitcoin-Qt`. Since #31844, there was also a `share/` containing the manpage for bitcoin-qt. This manpage is not useful to app users, and it is also causing code signing issues. Thus the directory should be removed when making the app.
Fixes https://github.com/bitcoin/bitcoin/issues/32052#issuecomment-2723007926
ACKs for top commit:
fanquake:
ACK 80b5e7f2cb7fbfbd724e1f52b00c0e72b79a200b
Tree-SHA512: fd15b7f99737484e40d31c583e01acc7470d038b0c584dfaefecc740811565ceee048913b6e5e37e7935b74d8100dc8323aed3f69d9a6baa5f434754009eb18c
5dfef6b9b379f51e69f2a358c05ae3c3e8a26e13 depends: remove NO_HARDEN option (fanquake)
Pull request description:
This was only needed to work around a (Libtool related iirc) Windows issue, when hardening was disabled. I can no-longer recreate this failure, so it'd be good to remove this Windows carveout.
ACKs for top commit:
davidgumberg:
crACK 5dfef6b9b379f51e
laanwj:
Code review ACK 5dfef6b9b379f51e69f2a358c05ae3c3e8a26e13
Tree-SHA512: 38657f09c537ba02ecaf0676d47087a835283cabfc81ad9b2d5e68858dcd7a610b6a1df6730920d40b48be2bbc55a45d6b8aea4364884b5f1c1bd12126940f5b
3c5d1a468199722da620f1f3d8ae3319980a46d5 Remove checkpoints (marcofleon)
632ae47372de90064f61e3e622d8da766d1d12de update comment on MinimumChainWork check (marcofleon)
Pull request description:
The headers presync logic (only downloading headers that lead to a chain with sufficient work, implemented in https://github.com/bitcoin/bitcoin/pull/25717) should be enough to prevent memory DoS using low-work headers. Therefore, we no longer have any use for checkpoints.
All checkpoints and checkpoint logic are removed in a single commit, to make it easy to revert if necessary.
Some previous discussion can be found in https://github.com/bitcoin/bitcoin/pull/25725. The conclusion at the time was that more testing of the presync logic was needed. Now that we have [unit](https://github.com/bitcoin/bitcoin/blob/master/src/test/headers_sync_chainwork_tests.cpp), [functional](https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_headers_sync_with_minchainwork.py), and [fuzz](https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/p2p_headers_presync.cpp) tests for this logic, it seems safe to move forward with checkpoint removal.
ACKs for top commit:
Sjors:
Code review ACK 3c5d1a468199722da620f1f3d8ae3319980a46d5
instagibbs:
reACK 3c5d1a468199722da620f1f3d8ae3319980a46d5
dergoegge:
ACK 3c5d1a468199722da620f1f3d8ae3319980a46d5
Tree-SHA512: 051a6f9b82cd0262f4d3be4403906812fc6d1be022731fac16bb1c02bca471f31dfc7fc4b834ab2469e8f087265a6d99e84a1d665823cda1b112363a8e8f337d
If we bump the mocktime before the node has successfully disconnected
the peer, the requests for both parents could be spread over
two GETDATAS, which would make the test fail.
a24419f8bed5e1145ce171dbbdad957750585471 contrib: Fix `gen-bitcoin-conf.sh`. (David Gumberg)
Pull request description:
In #31118, the format of bitcoind's `--help` output changed slightly in a way that breaks `gen-bitcoin-conf.sh`, modify the script to accommodate the new format, by starting after the line that says "Options:" and stripping the `-help` options and descriptions from the script output.
Before this PR, all options above `-help` were excluded from the example bitcoin.conf.
ACKs for top commit:
mabu44:
Tested ACK a24419f8bed5e1145ce171dbbdad957750585471
glozow:
ACK a24419f8bed5e1145ce171dbbdad957750585471
rkrux:
tACK a24419f8bed5e1145ce171dbbdad957750585471
BrandonOdiwuor:
crACK a24419f8bed5e1145ce171dbbdad957750585471
Tree-SHA512: 2ef697166d0b37b61ec1a20e357b91d611c932a0e453c4669f74ab69e6310ea1776dce09c1b77e82746072265763cb0c750e6df4c8b4a7d39068fc03f97b221b
