d168458d1ff987e0d741c75ac1d4b63ae0cfb7e7 scripted-diff: Remove unused chainparamsbase includes (TheCharlatan)
e9ee8aaf3acdf6dce2b339916d4c602484570050 Add missing definitions in prep for scripted diff (TheCharlatan)
ba8fc7d788932b25864fb260ca14983aa2398c23 refactor: Replace string chain name constants with ChainTypes (TheCharlatan)
401453df419af35957ec711423ac3d93ad512fe8 refactor: Introduce ChainType getters for ArgsManager (TheCharlatan)
bfc21c31b2186f7d30fc9a9ca7d6887ab61c6fb9 refactor: Create chaintype files (TheCharlatan)
Pull request description:
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is also a follow up to #26177.
It replaces pull request https://github.com/bitcoin/bitcoin/pull/27294, which just moved the constants to a new file, but did not re-declare them as enums.
The code move of the chain name constants out of the `chainparamsbase` to their own separate header allows the kernel `chainparams` to no longer include `chainparamsbase`. The `chainparamsbase` contain references to the `ArgsManager` and networking related options that should not belong to the kernel library. Besides this move, the constants are re-declared as enums with helper functions facilitating string conversions.
ACKs for top commit:
ryanofsky:
Code review ACK d168458d1ff987e0d741c75ac1d4b63ae0cfb7e7. Just suggested changes since last review.
Tree-SHA512: ac2fbe5cbbab4f52eae1e30af1f16700b6589eb4764c328a151a712adfc37f326cc94a65c385534c57d4bc92cc1a13bf1777d92bc924a20dbb30440e7380b316
This is a follow-up to previous commits moving the chain constants out
of chainparamsbase.
The script removes the chainparamsbase header in all files where it is
included, but not used. This is done by filtering against all defined
symbols of the header as well as its respective .cpp file.
The kernel chainparams now no longer relies on chainparamsbase.
-BEGIN VERIFY SCRIPT-
sed -i '/#include <chainparamsbase.h>/d' $( git grep -l 'chainparamsbase.h' | xargs grep -L 'CBaseChainParams\|CreateBaseChainParams\|SetupChainParamsBaseOptions\|BaseParams\|SelectBaseParams\|chainparamsbase.cpp' )
-END VERIFY SCRIPT-
This commit effectively moves the definition of these constants
out of the chainparamsbase to their own file.
Using the ChainType enums provides better type safety compared to
passing around strings.
The commit is part of an ongoing effort to decouple the libbitcoinkernel
library from the ArgsManager and other functionality that should not be
part of the kernel library.
fe49f06c0e91b96feb8d8f1bd478c3173f14782c doc: clarify PR 26076 release note (Sjors Provoost)
bd13dc2f46ea10302a928fcf0f53b7aed77ad260 Switch hardened derivation marker to h in descriptors (Sjors Provoost)
Pull request description:
This makes it easier to handle descriptor strings manually, especially when importing from another Bitcoin Core wallet.
For example the `importdescriptors` RPC call is easiest to use `h` as the marker: `'["desc": ".../0h/..."]'`, avoiding the need for escape characters. With this change `listdescriptors` will use `h`, so you can copy-paste the result, without having to add escape characters or switch `'` to 'h' manually.
Both markers can still be parsed.
The `hdkeypath` field in `getaddressinfo` is also impacted by this change, except for legacy wallets. The latter is to prevent accidentally breaking ancient software that uses our legacy wallet.
See discussion in #15740
ACKs for top commit:
achow101:
ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
darosior:
re-ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
Tree-SHA512: f78bc873b24a6f7a2bf38f5dd58f2b723e35e6b10e4d65c36ec300e2d362d475eeca6e5afa04b3037ab4bee0bf8ebc93ea5fc18102a2111d3d88fc873c08dc89
fa83fb31619c19a1a30b4181486601a944941b16 wallet: Use steady clock to calculate number of derive iterations (MarcoFalke)
fa2c099cec1780a498e198750be0cf5bf0ca315a wallet: Use steady clock to measure scanning duration (MarcoFalke)
fa976218044f3ff244abbd797b183a1408375c74 qt: Use steady clock to throttle GUI notifications (MarcoFalke)
fa1d8044abc2cd0f149a2d526b3b03441443cdb0 test: Use steady clock in index tests (MarcoFalke)
fa454dcb20b9e7943cc25e6eeea72912b5f1c7b5 net: Use steady clock in InterruptibleRecv (MarcoFalke)
Pull request description:
`GetTimeMillis` has multiple issues:
* It doesn't denote the underlying clock type
* It isn't type-safe
* It is used incorrectly in places that should use a steady clock
Fix all issues here.
ACKs for top commit:
willcl-ark:
ACK fa83fb3161
martinus:
Code review ACK fa83fb3161, also ran all tests. All usages of the steady_clock are just for duration measurements, so the change to a different epoch is ok.
Tree-SHA512: 5ec4fede8c7f97e2e08863c011856e8304f16ba30a68fdeb42f96a50a04961092cbe46ccf9ea6ac99ff5203c09f9e0924eb483eb38d7df0759addc85116c8a9f
bf77fc9cb45209b9c560208c65abc94209cd7919 [test] mempool full in package accept (glozow)
b51ebccc28e66c1822ab22d2d178be55c6618196 [validation] set PackageValidationState when mempool full (glozow)
563a2ee4f564c8ea5f8313d711b196e260568c04 [policy] disallow transactions under min relay fee, even in packages (glozow)
c4554fe894d7af8e666f5d424deccddf516713ef [test] package cpfp bumps parents <mempoolminfee but >=minrelaytxfee (glozow)
ac463e87df728689701810c3961155c49fdc5b31 [test util] mock mempool minimum feerate (glozow)
Pull request description:
Part of package relay, see #27463.
Note that this still allows packages to bump transactions that are below the dynamic mempool minimum feerate, which means this still solves the "mempool is congested and my presigned 1sat/vB tx is screwed" problem for all transactions.
On master, the package policy (only accessible through regtest-only RPC submitpackage) allows 0-fee (or otherwise below min relay feerate) transactions if they are bumped by a child. However, with default package limits, we don't yet have a DoS-resistant way of ensuring these transactions remain bumped throughout their time in the mempool. Primarily, the fee-bumping child may later be replaced by another transaction that doesn't bump the parent(s). The parent(s) could potentially stay bumped by other transactions, but not enough to ever be selected by the `BlockAssembler` (due to `blockmintxfee`).
For example, (tested [here](https://github.com/glozow/bitcoin/commits/26933-motivation)):
- The mempool accepts 24 below-minrelayfeerate transactions ("0-fee parents"), all bumped by a single high-fee transaction ("the fee-bumping child"). The fee-bumping child also spends a confirmed UTXO.
- Two additional children are added to each 0-fee parent. These children each pay a feerate slightly above the minimum relay feerate (e.g. 1.9sat/vB) such that, for each 0-fee parent, the total fees of its two children divided by the total size of the children and parent is above the minimum relay feerate.
- If a block template is built now, all transactions would be selected.
- A transaction replaces the the fee-bumping child, spending only the confirmed UTXO and not any of the outputs from the 0-fee parents.
- The 0-fee parents now each have 2 children. Their descendant feerates are above minrelayfeerate, which means that they remain in the mempool, even if the mempool evicts all below-minrelayfeerate packages.
- If a block template is built now, none of the 0-fee parents or their children would be selected.
- Even more low-feerate descendants can be added to these below-minrelayfeerate packages and they will not be evicted until they expire or the mempool reaches capacity.
Unless we have a DoS-resistant way of ensuring package CPFP-bumped transactions are always bumped, allowing package CPFP to bump below-minrelayfeerate transactions can result in these problematic situations. See #27018 which proposes a partial solution with some limitations, and contains discussion about potential improvements to eviction strategy. While no adequate solution exists, for now, avoid these situations by requiring all transactions to meet min relay feerate.
ACKs for top commit:
ajtowns:
reACK bf77fc9cb45209b9c560208c65abc94209cd7919
instagibbs:
re-ACK bf77fc9cb4
Tree-SHA512: 28940f41493a9e280b010284316fb8caf1ed7b2090ba9a4ef8a3b2eafc5933601074b142f4f7d4e3c6c4cce99d3146f5c8e1393d9406c6f2070dd41c817985c9
10a354f1740a5c1b913d0b6951e80fb5401ab43a test: prevent intermittent failures (Amiti Uttarwar)
Pull request description:
Follow up to #27214 - add an address to the tried table before the new table to make sure a new table collision is not possible.
ACKs for top commit:
mzumsande:
Code review ACK 10a354f1740a5c1b913d0b6951e80fb5401ab43a - the fix is what I suggested [here](https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1169169601) and should make these intermittent failures impossible.
Tree-SHA512: 24099f02e1915395130065af0ef6a2a1893955d222517d156d928765541d9c427da00172a9b5a540163f4d6aae93ca3882e8267eeb35ecc595d42178abc6191c
faa7144d3cf41e6410d942a3c485982ee65b3c6e fuzz: re-enable prioritisetransaction & analyzepsbt RPC (MarcoFalke)
Pull request description:
The linked issue seems fixed, so it should be fine to re-enable
ACKs for top commit:
dergoegge:
utACK faa7144d3cf41e6410d942a3c485982ee65b3c6e
Tree-SHA512: a681c726fceacc27ab5a03d455c7808d33f3cb11fe7d253d455526568af840b29f0c3c1d97c54785ef9277e7891a3aa742ac73ccd3cf115b7606eba50864aaa9
be55f545d53d44fdcf2d8ae802e9eae551d120c6 move-only: Extract common/args and common/config.cpp from util/system (TheCharlatan)
Pull request description:
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is part of a series of patches splitting up the `util/system` files. Its preceding pull request is https://github.com/bitcoin/bitcoin/pull/27254.
The pull request contains an extraction of ArgsManager related functions from util/system into their own common/ file.
The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See [doc/design/libraries.md](https://github.com/bitcoin/bitcoin/blob/master/doc/design/libraries.md) for more information on this rationale.
ACKs for top commit:
MarcoFalke:
re-ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6 🚲
ryanofsky:
Code review ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6. Just small cleanups since the last review.
hebasto:
ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 90eb03334af0155b823030b4f2ecf286d35058d700ee2ddbbaa445be19e31eb0fe982656f35bd14ecee3ad2c3d0db3746855cb8f3777eff7253713e42873e111
9f947fc3d4b779f017332135323b34e8f216f613 Use PoolAllocator for CCoinsMap (Martin Leitner-Ankerl)
5e4ac5abf54f8e6d6330df0c73119aa0cca4c103 Call ReallocateCache() on each Flush() (Martin Leitner-Ankerl)
1afca6b663bb54022afff193fd9d83856606b189 Add PoolResource fuzzer (Martin Leitner-Ankerl)
e19943f049ed8aa4f32a1d8440a9fbf160367f0f Calculate memory usage correctly for unordered_maps that use PoolAllocator (Martin Leitner-Ankerl)
b8401c3281978beed6198b2f9782b6a8dd35cbd7 Add pool based memory resource & allocator (Martin Leitner-Ankerl)
Pull request description:
A memory resource similar to `std::pmr::unsynchronized_pool_resource`, but optimized for node-based containers. The goal is to be able to cache more coins with the same memory usage, and allocate/deallocate faster.
This is a reimplementation of #22702. The goal was to implement it in a way that is simpler to review & test
* There is now a generic `PoolResource` for allocating/deallocating memory. This has practically the same API as `std::pmr::memory_resource`. (Unfortunately I cannot use std::pmr because libc++ simply doesn't implement that API).
* Thanks to sipa there is now a fuzzer for PoolResource! On a fast machine I ran it for ~770 million executions without finding any issue.
* The estimation of the correct node size is now gone, PoolResource now has multiple pools and just needs to be created large enough to have space for the unordered_map nodes.
I ran benchmarks with #22702, mergebase, and this PR. Frequency locked Intel i7-8700, clang++ 13.0.1 to reindex up to block 690000.
```sh
bitcoind -dbcache=5000 -assumevalid=00000000000000000002a23d6df20eecec15b21d32c75833cce28f113de888b7 -reindex-chainstate -printtoconsole=0 -stopatheight=690000
```
The performance is practically identical with #22702, just 0.4% slower. It's ~21% faster than master:
Note that on cache drops mergebase's memory doesnt go so far down because it does not free the `CCoinsMap` bucket array.
ACKs for top commit:
LarryRuane:
ACK 9f947fc3d4b779f017332135323b34e8f216f613
achow101:
re-ACK 9f947fc3d4b779f017332135323b34e8f216f613
john-moffett:
ACK 9f947fc3d4b779f017332135323b34e8f216f613
jonatack:
re-ACK 9f947fc3d4b779f017332135323b34e8f216f613
Tree-SHA512: 48caf57d1775875a612b54388ef64c53952cd48741cacfe20d89049f2fb35301b5c28e69264b7d659a3ca33d4c714d47bafad6fd547c4075f08b45acc87c0f45
17e705428ddf80c7a7f31fe5430d966cf08a37d6 doc: clarify new_only param for Select function (Amiti Uttarwar)
b0010c83a1b4a3d21719cb68e37faf9b1172522a bench: test select for a new table with only one address (Amiti Uttarwar)
9b91aae08579c77d2fd5506804c8e2e0cda0d274 bench: add coverage for addrman select with network parameter (Amiti Uttarwar)
22a4d1489c0678a90c00318203cfce61672f20b7 test: increase coverage of addrman select (without network) (Amiti Uttarwar)
a98e542e0c18f7cb2340179631806f14b07430c3 test: add addrman test for special case (Amiti Uttarwar)
5c8b4baff27e0ccd27fda6e915b956d1e8dd7ce2 tests: add addrman_select_by_network test (Amiti Uttarwar)
6b229284fd2209938ee8fdffed4d080395b3aa05 addrman: add functionality to select by network (Amiti Uttarwar)
26c3bf11e2487ed0ac578fb92619c148336003cb scripted-diff: rename local variables to match modern conventions (Amiti Uttarwar)
48806412e2bcd023b78fc05f6c9ce092360d1db1 refactor: consolidate select logic for new and tried tables (Amiti Uttarwar)
ca2a9c5f8f14b792a14e81f73b1910a4c8799b93 refactor: generalize select logic (Amiti Uttarwar)
052fbcd5a791855406141e85d32e42e297220fe9 addrman: Introduce helper to generalize looking up an addrman entry (Amiti Uttarwar)
9bf078f66c8f286e1ab5e34b8eeed7d80290a897 refactor: update Select_ function (Amiti Uttarwar)
Pull request description:
For the full context & motivation of this patch, see #27213
This is joint work with mzumsande.
This PR adds functionality to `AddrMan::Select` to enable callers to specify a network they are interested in.
Along the way, it refactors the function to deduplicate the logic, updates the local variables to match modern conventions, adds test coverage for both the new and existing `Select` logic, and adds bench tests for the worst case performance of both the new and existing `Select` logic.
This functionality is used in the parent PR.
ACKs for top commit:
vasild:
ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6
brunoerg:
re-ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6
ajtowns:
ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6
mzumsande:
Code Review ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6
Tree-SHA512: e99d1ce0c44a15601a3daa37deeadfc9d26208a92969ecffbea358d57ca951102d759734ccf77eacd38db368da0bf5b6fede3cd900d8a77b3061f4adc54e52d8
This is an extraction of ArgsManager related functions from util/system
into their own common file.
Config file related functions are moved to common/config.cpp.
The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from the ArgsManager. The ArgsManager belongs
into the common library, since the kernel library should not depend on
it. See doc/design/libraries.md for more information on this rationale.
`evhttp_uri_parse` can return a nullptr, for example when the URI
contains invalid characters (e.g. "%").
`GetQueryParameterFromUri` passes the output of `evhttp_uri_parse`
straight into `evhttp_uri_get_query`, which means that anyone calling
a REST endpoint in which query parameters are used (e.g. `rest_headers`)
can cause a segfault.
This bugfix is designed to be minimal and without additional behaviour change.
Follow-up work should be done to resolve this in a more general and robust way,
so not every endpoint has to handle it individually.
Avoid adding transactions below min relay feerate because, even if they
were bumped through CPFP when entering the mempool, we do not have a
DoS-resistant way of ensuring they always remain bumped. In the future,
this rule can be relaxed (e.g. to allow packages to bump 0-fee
transactions) if we find a way to do so.
3153e7d779ac284f86e433af033d63f13f361b6f [fuzz] Add HeadersSyncState target (dergoegge)
53552affca381cdb5103ecdbcc7f3fb562e66ac4 [headerssync] Make m_commit_offset protected (dergoegge)
Pull request description:
This adds a fuzz target for the `HeadersSyncState` class.
I am unsure how well this is able to cover the logic since it is just processing unserialized CBlockHeaders straight from the fuzz input (headers are sometimes made continuous). However, it does manage to get to the redownload phase so i thought it is better then not having fuzzing at all.
It would also be nice to fuzz the p2p logic that is using `HeadersSyncState` (e.g. `TryLowWorkHeadersSync`, `IsContinuationOfLowWorkHeadersSync`) but that likely requires some more work (refactoring👻).
ACKs for top commit:
mzumsande:
ACK 3153e7d779ac284f86e433af033d63f13f361b6f
Tree-SHA512: 8a4630ceeeb30e4eeabaa8eb5491d98f0bf900efe7cda07384eaac9f2afaccfbcaa979cc1cc7f0b6ca297a8f5c17a7759f94809dd87eb87d35348d847c83e8ab
This makes it easier to handle descriptor strings manually. E.g. an RPC call that takes an array of descriptors can now use '["desc": ".../0h/..."]'.
Both markers can still be parsed. The default for new descriptors is changed to h. In normalized form h is also used. For private keys the chosen marker is preserved in a round trip.
The hdkeypath field in getaddressinfo is also impacted by this change.
00e9b97f37e0bdf4c647236838c10b68b7ad5be3 refactor: Move fs.* to util/fs.* (TheCharlatan)
106b46d9d25b5228ef009fbbe6f9a7ae35090d15 Add missing fs.h includes (TheCharlatan)
b202b3dd6393b415fa68e18dc49c9431dc6b58b2 Add missing cstddef include in assumptions.h (TheCharlatan)
18fb36367a28819bd5ab402344802796a1248979 refactor: Extract util/fs_helpers from util/system (Ben Woosley)
Pull request description:
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". This commit was originally authored by empact and is taken from its parent PR #25152.
#### Context
There is an ongoing effort to decouple the `ArgsManager` used for command line parsing user-provided arguments from the libbitcoinkernel library (https://github.com/bitcoin/bitcoin/pull/25290, https://github.com/bitcoin/bitcoin/pull/25487, https://github.com/bitcoin/bitcoin/pull/25527, https://github.com/bitcoin/bitcoin/pull/25862, https://github.com/bitcoin/bitcoin/pull/26177, and https://github.com/bitcoin/bitcoin/pull/27125). The `ArgsManager` is defined in `system.h`. A similar pull request extracting functionality from `system.h` has been merged in https://github.com/bitcoin/bitcoin/pull/27238.
#### Changes
Next to providing better code organization, this PR removes some reliance of the tree of libbitcoinkernel header includes on `system.h` (and thus the `ArgsManager` definition) by moving filesystem related functions out of the `system.*` files.
There is already a pair of `fs.h` / `fs.cpp` in the top-level `src/` directory. They were not combined with the files introduced here, to keep the patch cleaner and more importantly because they are often included without the utility functions. The new files are therefore named `fs_helpers` and the existing `fs` files are moved into the util directory.
Further commits splitting more functionality out of `system.h` are still in #25152 and will be submitted in separate PRs once this PR has been processed.
ACKs for top commit:
hebasto:
ACK 00e9b97f37e0bdf4c647236838c10b68b7ad5be3
Tree-SHA512: 31422f148d14ba3c843b99b1550a6fd77c77f350905ca324f93d4f97b652246bc58fa9696c64d1201979cf88733e40be02d262739bb7d417cf22bf506fdb7666
faf8dc496e761a15956f8226d727f4bbab8dff82 fuzz: Remove legacy int parse fuzz tests (MarcoFalke)
Pull request description:
The fuzz tests checked that the result of the new function was equal to the legacy function. (Side note: The checks were incomplete, as evident by the follow-up fix in commit b5c9bb5cb9f4a8db57b33ef7399310c7d6de5822).
Given that they haven't found any issues in years (beside missing the above issue, that they couldn't catch), it seems time to remove them.
They may come in handy in the rare case that someone would want to modify `LocaleIndependentAtoi()` or `Parse*Int*()`, however that seems unlikely. Also, appropriate checks can be added then.
ACKs for top commit:
fanquake:
ACK faf8dc496e761a15956f8226d727f4bbab8dff82
dergoegge:
ACK faf8dc496e761a15956f8226d727f4bbab8dff82
Tree-SHA512: 4ec88b9fa8ba49a923b0604016f0f471b3c9b9e0ba6c5c3dc4e20503c6994789921e7221d9ec467a2a37a73f21a70ba51ba3370ed5ad311dee989e218290b29a
cd0c8eeb0940790b6ba83786d1c9e362d4dc4829 [net] Pass nRecvFloodSize to CNode (dergoegge)
860402ef2ed728ef096dda4e65e77d566782209f [net] Remove trivial GetConnectionType() getter (dergoegge)
b5a85b365a4abd98176b0935015dbb502cc3e6f6 [net] Delete CNetMessage copy constructor/assignment op (dergoegge)
Pull request description:
Follow-up PR for #27257
* Deletes the copy constructor/assignment operator of `CNetMessage`
* Removes trivial getter for the connection type
* Avoids passing `nRecvFloodSize` to CNode methods by passing it to `CNode` on creation
ACKs for top commit:
jnewbery:
utACK cd0c8eeb0940790b6ba83786d1c9e362d4dc4829
theStack:
ACK cd0c8eeb0940790b6ba83786d1c9e362d4dc4829
Tree-SHA512: 673a758668617f69fba77e61f0eaa1538da27a4849c82c98742436692baa2d7f001129af3e7a66b160e599d12109dac08137a146f10ff9b9ebdc5c2237311d41
In my benchmarks, using this pool allocator for CCoinsMap gives about
20% faster `-reindex-chainstate` with -dbcache=5000 with practically the
same memory usage. The change in max RSS changed was 0.3%.
The `validation_flush_tests` tests need to be updated because
memory allocation is now done in large pools instead of one node at a
time, so the limits need to be updated accordingly.
This frees up all associated memory with the map, not only the nodes.
This is necessary in preparation for using the PoolAllocator for
CCoinsMap, which does not actually free any memory on clear().
A memory resource similar to std::pmr::unsynchronized_pool_resource, but
optimized for node-based containers.
Co-Authored-By: Pieter Wuille <pieter@wuille.net>
3566aa7d495bb783bbd135726238d9f2a9e9f80e [net] Remove CNode friends (dergoegge)
3eac5e7cd1eda6ababb9af9cd72dae08d395993d [net] Add CNode helper for send byte accounting (dergoegge)
60441a3432df10f5d7c15c09c9569f27a793625b scripted-diff: [net] Rename CNode process queue members (dergoegge)
6693c499f7d48aaa1f527a1a860b943fc8442241 [net] Make cs_vProcessMsg a non-recursive mutex (dergoegge)
23d93526545c271b0eed2bd468681864c4213cce [net] Make CNode msg process queue members private (dergoegge)
897e342d6ec320c286753daef01d6eb9839e2c4d [net] Encapsulate CNode message polling (dergoegge)
cc5cdf877666c232a94f03faaf430cbeb6968372 [net] Deduplicate marking received message for processing (dergoegge)
ad44aa5c64d4ee5f31c867fda26350ab560575b7 [net] Add connection type getter to CNode (dergoegge)
Pull request description:
We should define clear interfaces between CNode, CConnman and PeerManager. This PR makes a small step in that direction by ending the friendship of CNode, CConnman and ConnmanTestMsg. CNode's message processing queue is made private in the process and its mutex is turned into a non-recursive mutex.
ACKs for top commit:
jnewbery:
utACK 3566aa7d495bb783bbd135726238d9f2a9e9f80e
vasild:
ACK 3566aa7d495bb783bbd135726238d9f2a9e9f80e
theStack:
re-ACK 3566aa7d495bb783bbd135726238d9f2a9e9f80e
brunoerg:
re-ACK 3566aa7d495bb783bbd135726238d9f2a9e9f80e
Tree-SHA512: 26b87da5054e32401b693b2904e9c5f40e35a53937c0b6cf44b8597034ad07bacf27d87cdffc54d3e7ccfebde4231ef30a38d326f88cc18133bbb34688ead567
The fs.* files are already part of the libbitcoin_util library. With the
introduction of the fs_helpers.* it makes sense to move fs.* into the
util/ directory as well.
This is an extraction of filesystem related functions from util/system
into their own utility file.
The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from the ArgsManager defined in system.h.
Moving these functions out of system.h allows including them from a
separate source file without including the ArgsManager definitions from
system.h.
95ad70ab652ddde7de65f633c36c1378b26a313a test: Default initialize `should_freeze` to `true` (Hennadii Stepanov)
cea50521fe810111a8a3c84ad14f944eafb5b658 refactor: Drop no longer used `swap` member functions (Hennadii Stepanov)
a87fb6bee5a7fb0879b3adea9a29997f1331acb0 clang-tidy: Fix modernize-use-default-member-init in `CScriptCheck` (Hennadii Stepanov)
b4bed5c1f98c0eed18f52fdcea11a420c10ed98d refactor: Drop no longer used `CScriptCheck()` default constructor (Hennadii Stepanov)
d8427cc28e3a9ac3319fb452b16661957c812b8f refactor: Use move semantics in `CCheckQueue::Loop` (Hennadii Stepanov)
9a0b5241396efe3b3ceb3931717c30bb94f99bfb clang-tidy, test: Fix bugprone-use-after-move in `Correct_Queue_range()` (Hennadii Stepanov)
04831fee6dca3eb86cd1d6b9ef879b296263fe35 refactor: Make move semantics explicit for callers (Hennadii Stepanov)
6c2d5972f3544c4f3e987828a99e88f27b62cf87 refactor: Use move semantics in `CCheckQueue::Add` (Hennadii Stepanov)
06820032142a75cc3c5b832045058bc6f6f74786 test, refactor: Avoid `CScriptCheck::swap` in `transaction_tests` (Hennadii Stepanov)
15209d97c6aad7d5c199fe007ad39b91c8ee6562 consensus, refactor: Avoid `CScriptCheck::swap` in `CheckInputScripts` (Hennadii Stepanov)
Pull request description:
This PR makes code more succinct and readable by using move semantics.
ACKs for top commit:
martinus:
re-ACK 95ad70ab652ddde7de65f633c36c1378b26a313a
achow101:
ACK 95ad70ab652ddde7de65f633c36c1378b26a313a
TheCharlatan:
re-ACK 95ad70ab65
MarcoFalke:
re-ACK 95ad70ab652ddde7de65f633c36c1378b26a313a 🚥
Tree-SHA512: adda760891b12d252dc9b823fe7c41eed660364b6fb1a69f17607d7a31eb0bbb82a80d154a7acfaa241b5de37d42a293c2b6e059f26a8e92d88d3a87c99768fb
