Merge #20429: refactor: replace (sizeof(a)/sizeof(a[0])) with C++17 s…

<pre>Merge #20429: refactor: replace (sizeof(a)/sizeof(a[0])) with C++17 std::size e829c9afbf75e930db6c3fe77a269b0af5e7a3ad refactor: replace sizeof(a)/sizeof(a[0]) by std::size (C++17) (Sebastian Falbesoner) 365539c84691d470b44d35df374d8c049f8c1192 refactor: init vectors via std::{begin,end} to avoid pointer arithmetic (Sebastian Falbesoner) 63d4ee1968144cc3d115f92baef95785abf813ac refactor: iterate arrays via C++11 range-based for loops if idx is not needed (Sebastian Falbesoner) Pull request description: This refactoring PR picks up the idea of #19626 and replaces all occurences of `sizeof(x)/sizeof(x[0])` (or `sizeof(x)/sizeof(*x)`, respectively) with the now-available C++17 [`std::size`]( (as [suggested by sipa](, making the macro `ARRAYLEN` obsolete. As preparation for this, two other changes are done to eliminate `sizeof(x)/sizeof(x[0])` usage: * all places where arrays are iterated via an index are changed to use C++11 range-based for loops If the index' only purpose is to access the array element (as [suggested by MarcoFalke]( * `std::vector` initializations are done via `std::begin` and `std::end` rather than using pointer arithmetic to calculate the end (also [suggested by MarcoFalke]( ACKs for top commit: practicalswift: cr ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad: patch looks correct fanquake: ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad MarcoFalke: review ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad &#127785; Tree-SHA512: b01d32c04b9e04d562b7717cae00a651ec9a718645047a90761be6959e0cc2adbd67494e058fe894641076711bb09c3b47a047d0275c736f0b2218e1ce0d193d</pre>

Merge #20942: [refactor] Move some net_processing globals into PeerMa…

<pre>Merge #20942: [refactor] Move some net_processing globals into PeerManagerImpl 6452190841f8da1cdaf899d064974136ab37659f net_processing: simplify MaybeSetPeerAsAnnouncingHeaderAndIDs args (Anthony Towns) 39c2a69bc28eb3e3b5fa15a3965773b459bbf7ad net_processing: move MaybeSetPeerAsAnnouncingHeadersAndIDs into PeerManagerImpl (Anthony Towns) 7b7117efd00acf7609e65d3b4fe5f76e400dda12 net_processing: simplify ProcessGetData and FindTxForGetData args (Anthony Towns) 34207b9004d2069a8fcb32758cd796143eccfb4d net_processing: move FindTxForGetData and ProcessGetData to PeerManagerImpl (Anthony Towns) d44084883adcf00f50d3d5a9e0c88e3a0b276817 net_processing: simplify PeerManageImpl method args (Anthony Towns) a490f0a056456d683dd8ef6f89a5af1a13792118 net_processing: move MarkBlockAs*, TipMayBeStale, FindNextBlocksToDL to PeerManagerImpl (Anthony Towns) 052d9bc7e52aea373a316f08d42460ead4ed16c8 net_processing: simplify AlreadyHaveTx args (Anthony Towns) eeac5062508c98fe58daaec471cdd27f3909b6ec net_processing: move AlreadyHaveTx into PeerManageImpl (Anthony Towns) 9781c08a33569370f191b30cc7e2ce9b5317eb3e net_processing: move some globals into PeerManagerImpl (Anthony Towns) Pull request description: Turns some globals into member variables, and simplifies the parameter list for some of net_processing's internal functions. Mostly just serves as a code cleanup at this point. ACKs for top commit: jnewbery: Code review ACK 6452190841f8da1cdaf899d064974136ab37659f ariard: Code Review ACK 6452190, changes are pretty straightforward. MarcoFalke: Concept ACK 6452190841 I have not reviewed this, but I left a comment &#128033; Tree-SHA512: 381361f9dbfeb851a5522ead3165ce1447a0f212ddea4b483aa38975559ee5ed03a4ba69c24fd69f36847a1eddfef05785f5cbb2fcec5fe50f8b336e8047c3b1</pre>

Merge #21064: refactor: use std::shared_mutex & remove Boost Thread

<pre>Merge #21064: refactor: use std::shared_mutex &amp; remove Boost Thread 060a2a64d40d75fecb60b7d2b9946a67e46aa6fc ci: remove boost thread installation (fanquake) 06e1d7d81d5a56d136c6fc88f09a2b0654a164f9 build: don't build or use Boost Thread (fanquake) 7097add83c8596f81be9edd66971ffd2486357eb refactor: replace Boost shared_mutex with std shared_mutex in sigcache (fanquake) 8e55981ef834490c438436719f95cbaf888c4914 refactor: replace Boost shared_mutex with std shared_mutex in cuckoocache tests (fanquake) Pull request description: This replaces `boost::shared_mutex` and `boost::unique_lock` with [`std::shared_mutex`]( &amp; [`std::unique_lock`]( Even though [some concerns were raised]( in #16684 with regard to `std::shared_mutex` being unsafe to use across some glibc versions, I still think this change is an improvement. As I mentioned in #21022, I also think trying to restrict standard library feature usage based on bugs in glibc is not only hard to do, but it's not currently clear exactly how we do that in practice (does it also extend to patching out use in our dependencies, should we be implementing more runtime checks for features we are using, when do we consider an affected glibc "old enough" not to worry about? etc). If you take a look through the [glibc bug tracker]( you'll no doubt find plenty of (active) bug reports for standard library code we already using. Obviously not to say we shouldn't try and avoid buggy code where possible. Two other points: [Cory mentioned in #21022]( &gt; It also seems reasonable to me to worry that boost hits the same underlying glibc bug, and we've just not happened to trigger the right conditions yet. Moving away from Boost to the standard library also removes the potential for differences related to Boosts configuration. Boost has multiple versions of `shared_mutex`, and what you end up using, and what it's backed by depends on: * The version of Boost. * The platform you're building for. * Which version of `BOOST_THREAD_VERSION` is defined: (2,3,4 or 5) default=2. (see [here]( for some of the differences). * Is `BOOST_THREAD_V2_SHARED_MUTEX` defined? (not by default). If so, you might get the ["less performant, but more robust"]( version of `shared_mutex`. A lot of these factors are eliminated by our use of depends, but users will have varying configurations. It's also not inconceivable to think that a distro, or some package manager might start defining something like `BOOST_THREAD_VERSION=3`. Boost tried to change the default from 2 to 3 at one point. With this change, we no longer use Boost Thread, so this PR also removes it from depends, the build system, CI etc. Previous similar PRs were #19183 &amp; #20922. The authors are included in the commits here. Also related to #21022 - pthread sanity checking. ACKs for top commit: laanwj: Code review ACK 060a2a64d40d75fecb60b7d2b9946a67e46aa6fc vasild: ACK 060a2a64d40d75fecb60b7d2b9946a67e46aa6fc Tree-SHA512: 572d14d8c9de20bc434511f20d3f431836393ff915b2fe9de5a47a02dca76805ad5c3fc4cceecb4cd43f3ba939a0508178c4e60e62abdbaaa6b3e8db20b75b03</pre>

Merge #21062: refactor: return MempoolAcceptResult from ATMP

<pre>Merge #21062: refactor: return MempoolAcceptResult from ATMP 53e716ea119658c28935fee24eb50090907c500e [refactor] improve style for touched code (gzhao408) 174cb5330af4b09f3a66974d3bae783ea43b190e [refactor] const ATMPArgs and non-const Workspace (gzhao408) f82baf0762f60c2ca5ffc339b095f9271d7c2f33 [refactor] return MempoolAcceptResult (gzhao408) 9db10a55061e09021ff8ea1d6637d99f7959035f [refactor] clean up logic in testmempoolaccept (gzhao408) Pull request description: This is the first 4 commits of #20833, and does refactoring only. It should be relatively simple to review, and offers a few nice things: - It makes accessing values that don't make sense (e.g. fee) when the tx is invalid an error. - Returning `MempoolAcceptResult` from ATMP makes the interface cleaner. The caller can get a const instead of passing in a mutable "out" param. - We don't have to be iterating through a bunch of lists for package validation, we can just return a `std::vector&lt;MempoolAcceptResult&gt;`. - We don't have to refactor all ATMP call sites again if/when we want to return more stuff from it. ACKs for top commit: MarcoFalke: ACK 53e716ea119658c28935fee24eb50090907c500e &#128191; jnewbery: Code review ACK 53e716ea119658c28935fee24eb50090907c500e ariard: Code Review ACK 53e716e, I did tweak a bit the touched paths to see if we had good test coverage. Didn't find holes. Tree-SHA512: fa6ec324a08ad9e6e55948615cda324cba176255708bf0a0a0f37cedb7a75311aa334ac6f223be7d8df3c7379502b1081102b9589f9a9afa1713ad3d9ab3c24f</pre>

Merge #21052: refactor: Replace fs::unique_path with GetUniquePath(pa…

<pre>Merge #21052: refactor: Replace fs::unique_path with GetUniquePath(path) calls 1bca2aa694cd85984c09699ae28daec313077462 Introduce GetUniquePath(base) helper method to replace boost::filesystem::unique_path() which is not available in std::filesystem. (Kiminuo) Pull request description: This PR makes it easier in #20744 to remove our dependency on the `boost::filesystem::unique_path()` function which does not have a direct equivalent in C++17. This PR attempts to re-implement `boost::filesystem::unique_path()` as `GetUniquePath(path)` but the implementations are not meant to be the same. Note: * Boost 1.75.0 implementation of `unique_path`: * In the previous implementation, I attempted to add: ```cpp fs::path GetUniquePath(const fs::path&amp; base) { FastRandomContext rnd; fs::path tmpFile = base / HexStr(rnd.randbytes(8)); return tmpFile; } ``` to `fs.cpp` but this leads to a circular dependency: "fs -&gt; random -&gt; logging -&gt; fs". That is why the modified implementation adds a new file. ACKs for top commit: laanwj: Code review ACK 1bca2aa694cd85984c09699ae28daec313077462 ryanofsky: Code review ACK 1bca2aa694cd85984c09699ae28daec313077462. It's a simple change and extra test coverage is nice Tree-SHA512: f324bdf0e254160c616b5033c3ece33d87db23eb0135acee99346ade7b5cf0d30f3ceefe359a25a8e9b53ba8e4419f459c2bdd369e10fc0152ce95031d1f221c</pre>