fa4cb13b52030c2e55c6bea170649ab69d75f758 test: [doc] Manually unify stale headers (MarcoFalke)
fa5f29774872d18febc0df38831a6e45f3de69cc scripted-diff: [doc] Unify stale copyright headers (MarcoFalke)
Pull request description:
Historically, the upper year range in file headers was bumped manually
or with a script.
This has many issues:
* The script is causing churn. See for example commit 306ccd4, or
drive-by first-time contributions bumping them one-by-one. (A few from
this year: https://github.com/bitcoin/bitcoin/pull/32008,
https://github.com/bitcoin/bitcoin/pull/31642,
https://github.com/bitcoin/bitcoin/pull/32963, ...)
* Some, or likely most, upper year values were wrong. Reasons for
incorrect dates could be code moves, cherry-picks, or simply bugs in
the script.
* The upper range is not needed for anything.
* Anyone who wants to find the initial file creation date, or file
history, can use `git log` or `git blame` to get more accurate
results.
* Many places are already using the `-present` suffix, with the meaning
that the upper range is omitted.
To fix all issues, this bumps the upper range of the copyright headers
to `-present`.
Further notes:
* Obviously, the yearly 4-line bump commit for the build system (c.f.
b537a2c02a9921235d1ecf8c3c7dc1836ec68131) is fine and will remain.
* For new code, the date range can be fully omitted, as it is done
already by some developers. Obviously, developers are free to pick
whatever style they want. One can list the commits for each style.
* For example, to list all commits that use `-present`:
`git log --format='%an (%ae) [%h: %s]' -S 'present The Bitcoin'`.
* Alternatively, to list all commits that use no range at all:
`git log --format='%an (%ae) [%h: %s]' -S '(c) The Bitcoin'`.
<!--
* The lower range can be wrong as well, so it could be omitted as well,
but this is left for a follow-up. A previous attempt was in
https://github.com/bitcoin/bitcoin/pull/26817.
ACKs for top commit:
l0rinc:
ACK fa4cb13b52030c2e55c6bea170649ab69d75f758
rkrux:
re-ACK fa4cb13b52030c2e55c6bea170649ab69d75f758
janb84:
ACK fa4cb13b52030c2e55c6bea170649ab69d75f758
Tree-SHA512: e5132781bdc4417d1e2922809b27ef4cf0abb37ffb68c65aab8a5391d3c917b61a18928ec2ec2c75ef5184cb79a5b8c8290d63e949220dbeab3bd2c0dfbdc4c5
1e94e562f76e6152dffb2a2d07dc3429137098b5 refactor: enable `readability-container-contains` clang-tidy rule (Lőrinc)
fd9f1accbda9e81b2c5290b2056b25f02152a607 Fix compilation for old Boost versions (Lőrinc)
Pull request description:
Replace the last few instances of `.count() != 0` and `.count() == 0` and bare `count()` patterns with the more expressive C++20 `.contains()` method:
* `std::set<std::string>` in `getblocktemplate` RPC;
* `std::map<std::string, ...>` in `transaction_tests`;
* other bare `std::unordered_set` and `std::map` count calls.
Also fixes https://github.com/bitcoin/bitcoin/issues/34101 by reverting `boost::multi_index::contains` calls not available in our minimum supported version.
With no remaining violations, enable the `readability-container-contains` clang-tidy check to prevent future regressions.
Follow-up to https://github.com/bitcoin/bitcoin/pull/33192
ACKs for top commit:
hebasto:
ACK 1e94e562f76e6152dffb2a2d07dc3429137098b5.
pablomartin4btc:
re-ACK 1e94e562f76e6152dffb2a2d07dc3429137098b5
janb84:
ACK 1e94e562f76e6152dffb2a2d07dc3429137098b5
rkrux:
re-ACK 1e94e562f76e6152dffb2a2d07dc3429137098b5
Tree-SHA512: d54a7821d319bf0d60b6c3a870917464a7d5b9279c6a86708c03a3516ec23bbf18f0e83de62b3b2b1607de96e1470f1144b4918d69a6c770e6b7e09863e7dbac
fa336053aada79d13cd771ce025857256814465e Move ci_exec to the Python script (MarcoFalke)
fa83555d163ff7fdcdaaa0e34bfa3eaa41fa6dfc ci: Require rsync to pass (MarcoFalke)
eeee02ea53dd1a3fb2eb62acd68fbd797d9b9ba8 ci: Untangle CI_EXEC bash function (MarcoFalke)
fa21fd1dc2e5649f8c4e7c04d28312beb51761fb ci: Move macos snippet under DANGER_RUN_CI_ON_HOST (MarcoFalke)
fa37559ac5b7bf83eefa30e7770ccae9fd19556b ci: Document the retry script in PATH (MarcoFalke)
666675e95fe823b7809f64508aea5b57b1867c19 ci: Move folder creation and docker kill to Python script (MarcoFalke)
Pull request description:
The remaining `ci/test/02_run_container.sh` is fine, but has a bunch of shellcheck SC2086 word splitting violations.
This is fine currently, because the only place that needed them had additional escaping, and all other commands happened to split fine on spaces.
However, this may change in the future. So fix it now, by rewriting it in Python, which is recommended in the dev notes.
ACKs for top commit:
frankomosh:
Code Review ACK [fa33605](fa336053aa)
m3dwards:
ACK fa336053aada79d13cd771ce025857256814465e
Tree-SHA512: 472decb13edca75566dffe49b9b3f554ab977fa60ec7902d5a060fe53381aee8606a10ff0c990a62ee2454dc6d9430cc064f58320b9043070b7bf08845413bf4
f480c1e7177744d11b058c3a9422975d7ec1af46 build: Update minimum required Boost version (Hennadii Stepanov)
Pull request description:
Building with Boost 1.73.0 has been [broken](https://github.com/bitcoin/bitcoin/pull/34095#pullrequestreview-3594758109) since https://github.com/bitcoin/bitcoin/pull/31122 was merged.
This PR updates the minimum required Boost version to 1.74.0.
According to https://repology.org/project/boost/versions, none of the major distros are affected by this change.
ACKs for top commit:
maflcko:
lgtm ACK f480c1e7177744d11b058c3a9422975d7ec1af46
l0rinc:
untested ACK f480c1e7177744d11b058c3a9422975d7ec1af46
pablomartin4btc:
utACK f480c1e7177744d11b058c3a9422975d7ec1af46
janb84:
ut ACK f480c1e7177744d11b058c3a9422975d7ec1af46
Tree-SHA512: 6af72a001a566fb5a7b60e23bdb9619e87f277a1a3928ceb304bd35b8b35f56e4d38f25983db9a8732ecdb957cec9850a0fcf6719f3a65d903872e63d80b4d7c
75bdb925f404f41874adf0fcefca0f1641fcb4e6 clusterlin: drop support for improvable chunking (simplification) (Pieter Wuille)
91399a79122cb6bb256e634049ac83f53154b64b clusterlin: remove unused MergeLinearizations (cleanup) (Pieter Wuille)
5ce280074512f79a4b3851d8d453f337fa97be46 clusterlin: randomize equal-feerate parts of linearization (privacy) (Pieter Wuille)
13aad26b78481f2abd6d5e3ae6218e46e4d0060a clusterlin: randomize various decisions in SFL (feature) (Pieter Wuille)
ddbfa4dfac7b30d0dc462b29bd4c89b9152d0381 clusterlin: keep FIFO queue of improvable chunks (preparation) (Pieter Wuille)
3efc94d6564deca4e5aaf5219adb71302f522657 clusterlin: replace cluster linearization with SFL (feature) (Pieter Wuille)
6a8fa821b80cf71e199b085c60d77f6ca533f6ee clusterlin: add support for loading existing linearization (feature) (Pieter Wuille)
da48ed9f348a8a93679fd623838059f04ac9e53c clusterlin: ReadLinearization for non-topological (tests) (Pieter Wuille)
c461259fb62982fd054cdf8e8fac8016e8dd9a04 clusterlin: add class implementing SFL state (preparation) (Pieter Wuille)
95bfe7d574cfc69bb35e386b88e2c3026b8a7315 clusterlin: replace benchmarks with SFL-hard ones (bench) (Pieter Wuille)
86dd550a9b706195a9d3d6124be3f7d569ccbffe clusterlin: add known-correct optimal linearization tests (tests) (Pieter Wuille)
Pull request description:
Part of cluster mempool: #30289.
This replaces the cluster linearization algorithm introduced in #30126 and #30286 (a combination of LIMO with candidate-set search), with a completely different algorithm: [spanning-forest linearization](https://delvingbitcoin.org/t/spanning-forest-cluster-linearization/1419/1), which appears to have much better performance for hard clusters. See [this post](https://delvingbitcoin.org/t/how-to-linearize-your-cluster/303/68) for a comparison between various linearization algorithms, and [this post](https://delvingbitcoin.org/t/how-to-linearize-your-cluster/303/73) for benchmarks comparing them. Replaying historical mempool data on it shows that it can effectively linearize every observed cluster up to 64 transactions optimally within tens of microseconds, though pathological examples can be created which take longer.
The algorithm is effectively a very specialized version of the [simplex algorithm](https://en.wikipedia.org/wiki/Simplex_algorithm) to the problem of finding high-feerate topological subsets of clusters, but modified to find all consecutive such subsets concurrently rather than just the first one. See the post above for how it is related.
It represents the cluster as partitioned into a set of chunks, each with a spanning tree of its internal dependencies connecting the transactions. Randomized improvements are made by selecting dependencies to add and remove to these spanning trees, merging and splitting chunks, until no more improvements are possible, or a computation budget is reached. Like simplex, it does not necessarily make progress in every step, and thus has no upper bound on its runtime to find optimal, but randomization makes long runtimes very unlikely, and additionally makes it hard to adversarially construct clusters in which the algorithm reliably makes bad choices.
ACKs for top commit:
instagibbs:
reACK 75bdb925f404f41874adf0fcefca0f1641fcb4e6
marcofleon:
reACK 75bdb925f404f41874adf0fcefca0f1641fcb4e6
Tree-SHA512: 189d85b34f0eb847562af7da724c61e39f0a785e24ebe2d4c8ee44698d02bd17842d699987d282a79bd1de30f50de28ec0f11d594ebbfa499f6a9b9ce35aecd8
Replace the last few instances of `.count() != 0` and `.count() == 0` and `.count()` patterns with the more expressive C++20 `.contains()` method:
* `std::set<std::string>` in `getblocktemplate` RPC;
* `std::map<std::string, ...>` in `transaction_tests`;
* other bare `std::unordered_set` and `std::map` count calls.
With no remaining violations, enable the `readability-container-contains`
clang-tidy check to prevent future regressions.
With MergeLinearizations() gone and the LIMO-based Linearize() replaced by SFL, we do not
need a class (LinearizationChunking) that can maintain an incrementally-improving chunk
set anymore.
Replace it with a function (ChunkLinearizationInfo) that just computes the chunks as
SetInfos once, and returns them as a vector. This simplifies several call sites too.
This places equal-feerate chunks (with no dependencies between them) in random
order in the linearization output, hiding information about DepGraph insertion
order from the output. Likewise, it randomizes the order of transactions within
chunks for the same reason.
This introduces a local RNG inside the SFL state, which is used to randomize
various decisions inside the algorithm, in order to make it hard to create
pathological clusters which predictably have bad performance.
The decisions being randomized are:
* When deciding what chunk to attempt to split, the queue order is
randomized.
* When deciding which dependency to split on, a uniformly random one is
chosen among those with higher top feerate than bottom feerate within
the chosen chunk.
* When deciding which chunks to merge, a uniformly random one among those
with the higher feerate difference is picked.
* When merging two chunks, a uniformly random dependency between them is
now activated.
* When making the state topological, the queue of chunks to process is
randomized.
This introduces a queue of chunks that still need processing, in both
MakeTopological() and OptimizationStep(). This is simultaneously:
* A preparation for introducing randomization, by allowing permuting the
queue.
* An improvement to the fairness of suboptimal solutions, by distributing
the work more fairly over chunks.
* An optimization, by avoiding retrying chunks over and over again which
are already known to be optimal.
This replaces the existing LIMO linearization algorithm (which internally uses
ancestor set finding and candidate set finding) with the much more performant
spanning-forest linearization algorithm.
This removes the old candidate-set search algorithm, and several of its tests,
benchmarks, and needed utility code.
The worst case time per cost is similar to the previous algorithm, so
ACCEPTABLE_ITERS is unchanged.
Rather than using an ad-hoc no-dependency copy of the graph when a potentially
non-topological linearization is needed in the clusterlin fuzz test, add this
directly as a feature in ReadLinearization().
This is preparation for a later commit where another use for such a function
is added.
This adds a data structure representing the optimization state for the spanning-forest
linearization algorithm (SFL), plus a fuzz test for its correctness.
This is preparation for switching over Linearize() to use this algorithm.
See https://delvingbitcoin.org/t/spanning-forest-cluster-linearization/1419 for
a description of the algorithm.
db2d39f642979f929261e5f1cd67f0c2f2ca045f fuzz: add subtest for re-downloading a previously pruned block (Eugene Siegel)
45f5b2dac330906368352a1c585183f0d75d779d fuzz: Add fuzzer for block index (Martin Zumsande)
c011e3aa542631a8857039df796ebf13a653e8a6 test: Wrap validation functions with TestChainstateManager (Martin Zumsande)
Pull request description:
This adds a fuzz target for the block index and various events in validation that interact with it.
It can create arbitrary tree-like structure of block indexes, simulating (so far) the following events:
- Adding a header
- Receiving the full block (may be valid or not)
- `ActivateBestChain()` - Reorging the chain to a new chain tip (possibly encountering invalid blocks on the way)
- Pruning a block in the best chain
- Receiving a previously pruned block again (`getblockfrompeer`)
It might be interesting / possible to extend this to more events, such as dealing with more than one chainstate (assumeutxo).
The test skips all actual validation of header/ block / transaction data by just simulating the outcome, and also doesn't interact with the data directory.
The main goal is to ensure the integrity of the block index tree in all fuzzed constellations, by calling `CheckBlockIndex()` at the end of each iteration.
Compared to #29158 this approach has a more limited scope (by skipping all actual validation), but it is fast - it doesn't do a full init sequence on each iteration, but "cleans up" after itself by resetting the global validation state after each iteration.
ACKs for top commit:
Crypt-iQ:
reACK db2d39f642
maflcko:
review ACK db2d39f642979f929261e5f1cd67f0c2f2ca045f 🍶
sedited:
Re-ACK db2d39f642979f929261e5f1cd67f0c2f2ca045f
Tree-SHA512: 76cd5f8f4d7d7258620b46d7438bad4508c3bdc98825b48b60f694b5a9838e2b2cf4967c0ead181f86f66f4939ddfe552471851b9d18f84f584c03dd7e09fc43
cd98caea438a61f1e2c6ea8331afa545478f8f94 Update ci.yml (Woolfgm)
Pull request description:
Updated actions/checkout from v5 to v6 in ci.yml workflow
ACKs for top commit:
fanquake:
ACK cd98caea438a61f1e2c6ea8331afa545478f8f94
Tree-SHA512: c2e8168400e0ef959d9a166070a91196d4b6abefda557b7a455fe4e5e6295d10132fb2c46885072379b844a9a9bd6adb25ac3301461db446c610967ca3363fbf
facd3d56ccbe2414a5f2b75be7132cd8b904f1e9 log: Use `__func__` for -logsourcelocations (MarcoFalke)
Pull request description:
The `-logsourcelocations` option was recently changed to print the full function signature, as a side-effect of moving toward `std::source_location` internally.
This is fine, but at least for me, it makes debugging functional test failures harder, because the log is just so massively verbose, with questionable benefit.
I think the historically used file name, line number, and plain `__func__` name are more than sufficient for `-logsourcelocations`.
So switch back to using that.
For reference, a verbose log may look like:
```
...
node0 2025-12-17T07:28:37.528146Z [init] [checkqueue.h:147] [CCheckQueue<T, R>::CCheckQueue(unsigned int, int) [with T = CScriptCheck; R = std::pair<ScriptError_t, std::__cxx11::basic_string<char> >]] Script verificatio
n uses 1 additional threads
...
```
I don't think there is value in printing stuff, like the (anon) namespace, the class template args, or the functionn (template) args. The following should be more than sufficient:
```
...
node0 2025-12-17T09:45:57.017122Z [init] [checkqueue.h:147] [CCheckQueue] Script verification uses 1 additional threads
...
ACKs for top commit:
ajtowns:
ACK facd3d56ccbe2414a5f2b75be7132cd8b904f1e9 -- those long signatures are terrible
stickies-v:
ACK facd3d56ccbe2414a5f2b75be7132cd8b904f1e9
Tree-SHA512: 22fd1f0074fc6e85754967f9219659f57c905005a2bea9176f0b439abec324d7e6c2f875c8951934a3b11ef7e9d7e38d5d5d307e2bd1e000bc27ee85635cd668
76e0e6087d0310ec31f43d751de60adf0c0a2faa qa: Account for errno not always being set for ConnectionResetError (Hodlinator)
Pull request description:
The lack of errno can cause unclear and long log output.
Issue can be triggered by:
```diff
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -263,6 +263,7 @@ std::string RequestMethodString(HTTPRequest::RequestMethod m)
/** HTTP request callback */
static void http_request_cb(struct evhttp_request* req, void* arg)
{
+ throw std::runtime_error{"Hello"};
evhttp_connection* conn{evhttp_request_get_connection(req)};
// Track active requests
{
```
and running a functional test such as *test/functional/feature_abortnode.py*.
`http.client.RemoteDisconnected` not specifying `errno` to `ConnectionResetError`-ctor: ce4b0ede16/Lib/http/client.py (L1556C9-L1556C29)
<details><summary>Before/after log examples</summary>
#### Log before
```
2025-11-14T20:53:05.272804Z TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
File "/home/hodlinator/bc/3/test/functional/test_framework/test_framework.py", line 138, in main
self.setup()
~~~~~~~~~~^^
File "/home/hodlinator/bc/3/test/functional/test_framework/test_framework.py", line 268, in setup
self.setup_network()
~~~~~~~~~~~~~~~~~~^^
File "/home/hodlinator/bc/3/./build/test/functional/feature_abortnode.py", line 21, in setup_network
self.setup_nodes()
~~~~~~~~~~~~~~~~^^
File "/home/hodlinator/bc/3/test/functional/test_framework/test_framework.py", line 381, in setup_nodes
self.start_nodes()
~~~~~~~~~~~~~~~~^^
File "/home/hodlinator/bc/3/test/functional/test_framework/test_framework.py", line 527, in start_nodes
node.wait_for_rpc_connection()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
File "/home/hodlinator/bc/3/test/functional/test_framework/test_node.py", line 326, in wait_for_rpc_connection
rpc.getblockcount()
~~~~~~~~~~~~~~~~~^^
File "/home/hodlinator/bc/3/test/functional/test_framework/coverage.py", line 50, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
File "/home/hodlinator/bc/3/test/functional/test_framework/authproxy.py", line 137, in __call__
response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/hodlinator/bc/3/test/functional/test_framework/authproxy.py", line 111, in _request
return self._get_response()
~~~~~~~~~~~~~~~~~~^^
File "/home/hodlinator/bc/3/test/functional/test_framework/authproxy.py", line 174, in _get_response
http_response = self.__conn.getresponse()
File "/nix/store/62fdlzq1x1ak2lsxp4ij7ip5k9nia3hc-python3-3.13.7/lib/python3.13/http/client.py", line 1430, in getresponse
response.begin()
~~~~~~~~~~~~~~^^
File "/nix/store/62fdlzq1x1ak2lsxp4ij7ip5k9nia3hc-python3-3.13.7/lib/python3.13/http/client.py", line 331, in begin
version, status, reason = self._read_status()
~~~~~~~~~~~~~~~~~^^
File "/nix/store/62fdlzq1x1ak2lsxp4ij7ip5k9nia3hc-python3-3.13.7/lib/python3.13/http/client.py", line 300, in _read_status
raise RemoteDisconnected("Remote end closed connection without"
" response")
http.client.RemoteDisconnected: Remote end closed connection without response
```
#### Log after
```
2025-11-14T20:48:10.552126Z TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
File "/home/hodlinator/bc/3/test/functional/test_framework/test_framework.py", line 138, in main
self.setup()
~~~~~~~~~~^^
File "/home/hodlinator/bc/3/test/functional/test_framework/test_framework.py", line 268, in setup
self.setup_network()
~~~~~~~~~~~~~~~~~~^^
File "/home/hodlinator/bc/3/./build/test/functional/feature_abortnode.py", line 21, in setup_network
self.setup_nodes()
~~~~~~~~~~~~~~~~^^
File "/home/hodlinator/bc/3/test/functional/test_framework/test_framework.py", line 381, in setup_nodes
self.start_nodes()
~~~~~~~~~~~~~~~~^^
File "/home/hodlinator/bc/3/test/functional/test_framework/test_framework.py", line 527, in start_nodes
node.wait_for_rpc_connection()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
File "/home/hodlinator/bc/3/test/functional/test_framework/test_node.py", line 316, in wait_for_rpc_connection
raise FailedToStartError(self._node_msg(
f'bitcoind exited with status {self.process.returncode} during initialization. {str_error}'))
test_framework.test_node.FailedToStartError: [node 0] bitcoind exited with status -6 during initialization. terminate called after throwing an instance of 'std::runtime_error'
what(): Hello
************************
```
Note how even the C++ exception message is now included.
</details>
ACKs for top commit:
maflcko:
review ACK 76e0e6087d0310ec31f43d751de60adf0c0a2faa 🌬
furszy:
Tested ACK 76e0e6087d0310ec31f43d751de60adf0c0a2faa
l0rinc:
untested code review ACK 76e0e6087d0310ec31f43d751de60adf0c0a2faa
Tree-SHA512: 55a83d664624932b919ab2a5b6369121db448d27628029f21c5df297892dd56d179d710ad744f6407b51aa576fb6905a38bbc29885c534ec20704c22717a0880
caf4843a59a9d2512d69f8fd88a9672112bd80ac fuzz: doc: remove any mention to address_deserialize_v2 (brunoerg)
Pull request description:
We don't have `address_deserialize_v2` target anymore since fac81affb527132945773a5315bd27fec61ec52f (we used to have `address_deserialize_v1_notime`, `address_deserialize_v1_withtime` and `address_deserialize_v2` but now we only have a single `address_deserialize` target) so it removes any mention to it.
ACKs for top commit:
maflcko:
review ACK caf4843a59a9d2512d69f8fd88a9672112bd80ac 🎾
marcofleon:
ACK caf4843a59a9d2512d69f8fd88a9672112bd80ac
Tree-SHA512: 539d69edbfe4ca11eb0701ed5c789ad81976e3e85e8a229e39e9dc1b1c72264f01d10a1c16d0a3bb4a354794412dc8b625298f4f72430905a00b65faeaa37d6b
d9319b06cf82664d55f255387a348135fd7f91c7 refactor: unify container presence checks - non-trivial counts (Lőrinc)
039307554eb311ce41648d1f9a12b543f480f871 refactor: unify container presence checks - trivial counts (Lőrinc)
8bb9219b6301215f53e43967d17445aaf1b81090 refactor: unify container presence checks - find (Lőrinc)
Pull request description:
### Summary
Instead of counting occurrences in sets and maps, the C++20 `::contains` method expresses the intent unambiguously and can return early on first encounter.
### Context
Applied clang‑tidy's [readability‑container‑contains](https://clang.llvm.org/extra/clang-tidy/checks/readability/container-contains.html) check, though many cases required manual changes since tidy couldn't fix them automatically.
### Changes
The changes made here were:
| From | To |
|------------------------|------------------|
| `m.find(k) == m.end()` | `!m.contains(k)` |
| `m.find(k) != m.end()` | `m.contains(k)` |
| `m.count(k)` | `m.contains(k)` |
| `!m.count(k)` | `!m.contains(k)` |
| `m.count(k) == 0` | `!m.contains(k)` |
| `m.count(k) != 1` | `!m.contains(k)` |
| `m.count(k) == 1` | `m.contains(k)` |
| `m.count(k) < 1` | `!m.contains(k)` |
| `m.count(k) > 0` | `m.contains(k)` |
| `m.count(k) != 0` | `m.contains(k)` |
> Note that `== 1`/`!= 1`/`< 1` only apply to simple [maps](https://en.cppreference.com/w/cpp/container/map/contains)/[sets](https://en.cppreference.com/w/cpp/container/set/contains) and had to be changed manually.
There are many other cases that could have been changed, but we've reverted most of those to reduce conflict with other open PRs.
-----
<details>
<summary>clang-tidy command on Mac</summary>
```bash
rm -rfd build && \
cmake -B build \
-DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \
-DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \
-DCMAKE_OSX_SYSROOT="$(xcrun --show-sdk-path)" \
-DCMAKE_C_FLAGS="-target arm64-apple-macos11" \
-DCMAKE_CXX_FLAGS="-target arm64-apple-macos11" \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_FOR_FUZZING=ON
"$(brew --prefix llvm)/bin/run-clang-tidy" -quiet -p build -j$(nproc) -checks='-*,readability-container-contains' | grep -v 'clang-tidy'
```
</details>
Note: this is a take 2 of https://github.com/bitcoin/bitcoin/pull/33094 with fewer contentious changes.
ACKs for top commit:
optout21:
reACK d9319b06cf82664d55f255387a348135fd7f91c7
sedited:
ACK d9319b06cf82664d55f255387a348135fd7f91c7
janb84:
re ACK d9319b06cf82664d55f255387a348135fd7f91c7
pablomartin4btc:
re-ACK d9319b06cf82664d55f255387a348135fd7f91c7
ryanofsky:
Code review ACK d9319b06cf82664d55f255387a348135fd7f91c7. I manually reviewed the full change, and it seems there are a lot of positive comments about this and no more very significant conflicts, so I will merge it shortly.
Tree-SHA512: e4415221676cfb88413ccc446e5f4369df7a55b6642347277667b973f515c3c8ee5bfa9ee0022479c8de945c89fbc9ff61bd8ba086e70f30298cbc1762610fe1
fa904fc683c0892eb800ddb986fdc0c646721077 lint: Remove confusing, redundant, and brittle lint-spelling (MarcoFalke)
Pull request description:
`codespell` was a fun experiment. However, it has many issues, when used in this project:
* The number of false-positives and true-positives are in the same ballpark. There are also many false-negatives, so the overall net-benefit is questionable.
* There is often confusion around spelling errors leading to a failing CI (they do not, which was intended).
* LLMs released this year are capable to detect typos with less false-positives and less false-negatives, so the `codespell` integration is a bit redundant in that sense.
Fix all issues by removing it.
Going forward, anyone is free to continue to use `codespell`, or any LLM, or any other tool, locally. Also, DrahtBot has the LLM typo linter integrated in the summary comment. I think the options are plenty, and are more than sufficient for now.
ACKs for top commit:
l0rinc:
ACK fa904fc683c0892eb800ddb986fdc0c646721077
rkrux:
ACK fa904fc683c0892eb800ddb986fdc0c646721077
pablomartin4btc:
ACK fa904fc683c0892eb800ddb986fdc0c646721077
Tree-SHA512: 5e2008a77c2c313605f30d73286111eba034a2a6bb2a0a48e2f77ec6ccc7afaa274e00bbfcb727be0ac5e547b8ae9c801d30c43589b0cad2099565e6716b9ec7
5ac35795206d252c9f464e967b84521ddaad38f1 refactor: Add compile-time-checked hex txid (rustaceanrob)
Pull request description:
Suggested by l0rinc as a comment in #34004.
There are tests that utilize `FromHex` that will only fail during runtime if malformed. Adds a compile time constructor that can be caught by LSPs.
ACKs for top commit:
l0rinc:
ACK 5ac35795206d252c9f464e967b84521ddaad38f1
maflcko:
review ACK 5ac35795206d252c9f464e967b84521ddaad38f1 🦎
rkrux:
crACK 5ac35795206d252c9f464e967b84521ddaad38f1
Tree-SHA512: b0bae2bf0b8cd8c9a90765a14c46146313cf8b224a29d58a253e65ca95c4205c0beddea9c49ae58901e72c8c5202b91695d074ffb1c48e448d2e5606eb1bd5b4
fa5ed16aa4d9dbe3ed47cb53f3cb15b0685a2b96 move-only: MAX_BLOCK_TIME_GAP to src/qt (MarcoFalke)
Pull request description:
`MAX_BLOCK_TIME_GAP` was used in some incorrect heuristics, which were removed in commit e30b6ea194fee3bb95a45e7b732a99566b88f1f5.
This leaves a single module in src/qt using the constant.
Instead of exposing it in a central kernel header, just move it to the single gui module that uses it.
ACKs for top commit:
sedited:
ACK fa5ed16aa4d9dbe3ed47cb53f3cb15b0685a2b96
hebasto:
ACK fa5ed16aa4d9dbe3ed47cb53f3cb15b0685a2b96, I have reviewed the code and it looks OK.
Tree-SHA512: d0e0e5257f6585d793bfed118d61a3e5d56b2be397fa3b09b34db64e3e018eba9f223cd56541d258b422119fdd7501f07cd3bb8ad5dc28b535922aa21ea76fa6
e7e51952dc24531932b6c06e4599be3a3d6bede8 contrib: Avoid outputting binary data to TTY (Hodlinator)
Pull request description:
Verify that we wouldn't be writing encoded asmap binary data directly to the TTY since it is the default but makes no sense. (Having stdout as default does make sense when piping to other applications however).
Found while exploring the ASMap data pipeline (https://github.com/asmap/asmap-data/pull/38#pullrequestreview-3547352533) from Kartograf into Bitcoin Core.
ACKs for top commit:
fjahr:
tACK e7e51952dc24531932b6c06e4599be3a3d6bede8
sipa:
ACK e7e51952dc24531932b6c06e4599be3a3d6bede8
Tree-SHA512: e1ae1ee129715471cbb824268e68cec267d159d4073297af35c06eadfb6b98eeae040beaafeb6489c2853ea9b83cd04471bcd0b27f0ae8fcb377e6e10b4ae6c5
faa8ee62f5c1606217fbe9eacdd504ec133920a4 ci: Pin native tests on cross-builds to same commit (MarcoFalke)
Pull request description:
After commit 13809b867ad980a12f886316b18bb2d3d2848ac2, the native tests may check out a different commit than the cross-build task that produced the artefacts they run on.
Obviously, this may lead to test failures.
Fix it, by first determining a fixed commit, to be used for both the build and the native tests.
An alternative could be to fully or partially revert 13809b867ad980a12f886316b18bb2d3d2848ac2, but that comes again with the downsides making it harder to detect silent merge conflicts by re-running CI, or clearing unrelated and fixed intermittent test issues by re-running CI. Then, the only alternative would be to close and re-open the pull request.
ACKs for top commit:
janb84:
ACK faa8ee62f5c1606217fbe9eacdd504ec133920a4
ryanofsky:
Code review ACK faa8ee62f5c1606217fbe9eacdd504ec133920a4. Thanks for the naming & display updates since last review!
hodlinator:
crACK faa8ee62f5c1606217fbe9eacdd504ec133920a4
Tree-SHA512: 01391cdfad34e3f2f5b3a6247b1aeb412d023e368bc17572aa66324688439786e79e263288276053ffcfa8521635b07339dbf087b8a30d6670373556a77c22ee
59b93f11e8600d5224359f4d05619c0f56aef1e6 rest: print also HTTP response reason in case of an error (Roman Zeyde)
7fe94a04934a89b63f1248cb46d59f0ab45439b5 rest: add a test for unsuported `/blockpart/` request type (Roman Zeyde)
55d0d19b5c02a65d8dfafd99f352769224ab51a4 rest: deduplicate `interface_rest.py` negative tests (Roman Zeyde)
89eb531024d9921f5c825d390b90c0a7bd3756cc rest: update release notes for `/blockpart/` endpoint (Roman Zeyde)
41118e17f87561afc8cbe1f3dd528624f06906a7 blockstorage: simplify partial block read validation (Roman Zeyde)
599effdeab4d6687da783de04f8edf1d88959169 rest: reformat `uri_prefixes` initializer list (Roman Zeyde)
Pull request description:
The commits below should resolve a few leftovers from #33657.
ACKs for top commit:
l0rinc:
ACK 59b93f11e8600d5224359f4d05619c0f56aef1e6
hodlinator:
re-ACK 59b93f11e8600d5224359f4d05619c0f56aef1e6
Tree-SHA512: ae45e08edd315018e11283b354fb32f9658f5829c956554dc662a81c2e16397def7c3700e6354e0a91ff03c850def35638a69ec2668b7c015d25d6fed42b92bb
This imitates the use of the getblockfrompeer rpc.
Note that currently pruning is limited to blocks in the active chain.
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
This fuzz target creates arbitrary tree-like structure of indices,
simulating the following events:
- Adding a header to the block tree db
- Receiving the full block (may be valid or not)
- Reorging to a new chain tip (possibly encountering invalid blocks on
the way)
- pruning
The test skips all actual validation of header/ block / transaction data
by just simulating the outcome, and also doesn't interact with the data directory.
The main goal is to test the integrity of the block index tree in
all fuzzed constellations, by calling CheckBlockIndex()
at the end of each iteration.
7e9de20c0c144f5ccea5efe6d90601dd72bc7461 fuzz: exercise `ComputeMerkleRoot` without mutated parameter (Lőrinc)
Pull request description:
The `mutated` parameter in `ComputeMerkleRoot` unlocks a different path that was always exercised in the fuzz test.
Adjusted to be fuzzer to pass `nullptr` as well to make sure that path is also tested: 24ed820d4f/src/consensus/merkle.cpp (L49-L53)
Follow-up to https://github.com/bitcoin/bitcoin/pull/33805#discussion_r2589073735
ACKs for top commit:
frankomosh:
ACK [7e9de20](7e9de20c0c)
hodlinator:
ACK 7e9de20c0c144f5ccea5efe6d90601dd72bc7461
sedited:
ACK 7e9de20c0c144f5ccea5efe6d90601dd72bc7461
Tree-SHA512: bf27029ac04003447b24a95544ec863f9ceca6c28d51ea811dd6ca2b412a2a780bb9fdbcdc82719f39dd710a746eb2446263e8377d67a8be52a1694571d03498
