aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2023-08-06Merge bitcoin/bitcoin#27213: p2p: Diversify automatic outbound connections ↵fanquake
with respect to networks 1b52d16d07be3b5d968157913f04d9cd1e2d3678 p2p: network-specific management of outbound connections (Martin Zumsande) 65cff00ceea48ac8a887ffea79aedb4251aa097f test: Add test for outbound protection by network (Martin Zumsande) 034f61f83b9348664d868933dbbfd8f9f8882168 p2p: Protect extra full outbound peers by network (Martin Zumsande) 654d9bc27647fb3797001472e2464dededb45d3f p2p: Introduce data struct to track connection counts by network (Amiti Uttarwar) Pull request description: This is joint work with mzumsande. This is a proposal to diversify outbound connections with respect to reachable networks. The existing logic evaluates peers for connection based purely on the frequency of available addresses in `AddrMan`. This PR adds logic to automatically connect to alternate reachable networks and adds eviction logic that protects one existing connection to each network. For instance, if `AddrMan` is populated primarily with IPv4 and IPv6 addresses and only a handful of onion addresses, it is likely that we won't establish any automatic outbound connections to Tor, even if we're capable of doing so. For smaller networks like CJDNS, this is even more of an issue and often requires adding manual peers to ensure regularly being connected to the network. Connecting to multiple networks improves resistance to eclipse attacks for individual nodes. It also benefits the entire p2p network by increasing partition resistance and privacy in general. The automatic connections to alternate networks is done defensively, by first filling all outbound slots with random addresses (as in the status quo) and then adding additional peers from reachable networks the node is currently not connected to. This approach ensures that outbound slots are not left unfilled while attempting to connect to a network that may be unavailable due to a technical issue or misconfiguration that bitcoind cannot detect. Once an additional peer is added and we have one more outbound connection than we want, outbound eviction ensures that peers are protected if they are the only ones for their network. Manual connections are also taken into account: If a user already establishes manual connections to a trusted peer from a network, there is no longer a need to make extra efforts to ensure we also have an automatic connection to it (although this may of course happen by random selection). ACKs for top commit: naumenkogs: ACK 1b52d16d07be3b5d968157913f04d9cd1e2d3678 vasild: ACK 1b52d16d07be3b5d968157913f04d9cd1e2d3678 Tree-SHA512: 5616c038a5fbb868d4c46c5963cfd53e4599feee25db04b0e18da426d77d22e0994dc4e1da0b810f5b457f424ebbed3db1704f371aa6cad002b3565b20170ec0
2023-08-04Merge bitcoin/bitcoin#28213: scripted-diff: Specify Python major version ↵fanquake
explicitly on Windows 6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91 scripted-diff: Specify Python major version explicitly on Windows (Hennadii Stepanov) Pull request description: On Windows, it is the accepted practice to use `py.exe` launcher: - https://learn.microsoft.com/en-us/windows/python/faqs#what-is-py-exe- - https://docs.python.org/3/using/windows.html#python-launcher-for-windows One of its features is the correct handling of shebang lines like the one we use: `#!/usr/bin/env python3`. However, Windows OS app execution aliases might [interfere](https://learn.microsoft.com/en-us/windows/python/faqs#why-does-running-python-exe-open-the-microsoft-store-) with the launcher's behaviour. Such aliases are enabled on Windows 11 by default: ![image](https://github.com/bitcoin/bitcoin/assets/32963518/407837ec-e89a-4bc1-98b1-db983002065a) For example, on a fresh Windows 11 Pro installation with the Python installed from the [Chocolatey](https://community.chocolatey.org/packages/python/3.11.4) package manager, one will get the following error: ``` >py -3 test\functional\rpc_signer.py 2023-08-03T19:41:13.353000Z TestFramework (INFO): PRNG seed is: 2694758731106548661 2023-08-03T19:41:13.353000Z TestFramework (INFO): Initializing test directory C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3 2023-08-03T19:41:14.538000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "C:\Users\hebasto\bitcoin\test\functional\test_framework\util.py", line 140, in try_rpc fun(*args, **kwds) File "C:\Users\hebasto\bitcoin\test\functional\test_framework\coverage.py", line 50, in __call__ return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\hebasto\bitcoin\test\functional\test_framework\authproxy.py", line 129, in __call__ raise JSONRPCException(response['error'], status) test_framework.authproxy.JSONRPCException: RunCommandParseJSON error: process(py C:\Users\hebasto\bitcoin\test\functional\mocks\signer.py enumerate) returned 9009: Python was not found; run without arguments to install from the Microsoft Store, or disable this shortcut from Settings > Manage App Execution Aliases. (-1) During handling of the above exception, another exception occurred: Traceback (most recent call last): File "C:\Users\hebasto\bitcoin\test\functional\test_framework\test_framework.py", line 131, in main self.run_test() File "C:\Users\hebasto\bitcoin\test\functional\rpc_signer.py", line 72, in run_test assert_raises_rpc_error(-1, 'fingerprint not found', File "C:\Users\hebasto\bitcoin\test\functional\test_framework\util.py", line 131, in assert_raises_rpc_error assert try_rpc(code, message, fun, *args, **kwds), "No exception raised" ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\hebasto\bitcoin\test\functional\test_framework\util.py", line 146, in try_rpc raise AssertionError( AssertionError: Expected substring not found in error message: substring: 'fingerprint not found' error message: 'RunCommandParseJSON error: process(py C:\Users\hebasto\bitcoin\test\functional\mocks\signer.py enumerate) returned 9009: Python was not found; run without arguments to install from the Microsoft Store, or disable this shortcut from Settings > Manage App Execution Aliases. '. 2023-08-03T19:41:14.592000Z TestFramework (INFO): Stopping nodes 2023-08-03T19:41:14.799000Z TestFramework (WARNING): Not cleaning up dir C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3 2023-08-03T19:41:14.799000Z TestFramework (ERROR): Test failed. Test logging available at C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3/test_framework.log 2023-08-03T19:41:14.799000Z TestFramework (ERROR): 2023-08-03T19:41:14.799000Z TestFramework (ERROR): Hint: Call C:\Users\hebasto\bitcoin\test\functional\combine_logs.py 'C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3' to consolidate all logs 2023-08-03T19:41:14.799000Z TestFramework (ERROR): 2023-08-03T19:41:14.799000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log. 2023-08-03T19:41:14.799000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues 2023-08-03T19:41:14.799000Z TestFramework (ERROR): ``` This PR resolves this issue by explicitly specifying the Python major version and makes testing of self-compiled binaries more straightforward. ACKs for top commit: MarcoFalke: lgtm ACK 6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91 stickies-v: utACK 6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91 Tree-SHA512: 5681141e222bc833c6250cb79fe3a1c8e02255eb2c86010bc0f8239afcdfed784ed7788c8579209d931bd357f58d5655cf33ffeb2f46b1879f37cdc30e7a7c91
2023-08-04Merge bitcoin/bitcoin#28203: refactor: serialization simplificationsfanquake
f054bd072afb72d8dae7adc521ce15c13b236700 refactor: use "if constexpr" in std::vector's Unserialize() (Martin Leitner-Ankerl) 088caa68fb8efd8624709d643913b8a7e1218f8a refactor: use "if constexpr" in std::vector's Serialize() (Martin Leitner-Ankerl) 0fafaca4d3bbf0c0b5bfe1ec617ab15252ea51e6 refactor: use "if constexpr" in prevector's Unserialize() (Martin Leitner-Ankerl) c8839ec5cd81ba9ae88081747c49ecc758973dd1 refactor: use "if constexpr" in prevector's Serialize() (Martin Leitner-Ankerl) 1403d181c106bc271ad2522adebde07c7850069b refactor: use fold expressions instead of recursive calls in UnserializeMany() (Martin Leitner-Ankerl) bd08a008b42dac921bd9c031637e378899c1cd1d refactor: use fold expressions instead of recursive calls in SerializeMany() (Martin Leitner-Ankerl) Pull request description: This simplifies the serialization code a bit and should also make it a bit faster. * use fold expressions instead of recursive calls. This simplifies the code, makes it most likely faster because it reduces the number of function calls, and compiles faster because there are fewer template instantiations. * use `if constexpr` instead of unnecessarily creating a temporary object only to call the right overload. This is used for `std::vector` and `prevector` serialization. ACKs for top commit: MarcoFalke: only change is to add a missing `&`. lgtm, re-ACK f054bd072afb72d8dae7adc521ce15c13b236700 📦 jonatack: ACK f054bd072afb72d8dae7adc521ce15c13b236700 sipa: utACK f054bd072afb72d8dae7adc521ce15c13b236700 john-moffett: ACK f054bd072afb72d8dae7adc521ce15c13b236700 Tree-SHA512: 0417bf2d6be486c581732297945449211fc3481bac82964e27628b38ef55a47dfa58d730148aeaf1b19fa8eb1076489cc646ceebb178162a9afa59034601501d
2023-08-03p2p: network-specific management of outbound connectionsMartin Zumsande
Diversify outbound connections with respect to networks: Every ~5 minutes, try to add an extra connection to a reachable network which we currently don't have a connection to. This is done defensively - only try management with respect to networks after all existing outbound slots are filled. The resulting situation with an extra outbound peer will be handled by the extra outbound eviction logic, which protects peers from eviction if they are the only ones for their network. Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
2023-08-03test: Add test for outbound protection by networkMartin Zumsande
Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
2023-08-03p2p: Protect extra full outbound peers by networkMartin Zumsande
If a peer is the only one of its network, protect it from eviction. This improves the diversity of outbound connections with respect to reachable networks. Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
2023-08-03p2p: Introduce data struct to track connection counts by networkAmiti Uttarwar
Connman uses this new map to keep a count of active OUTBOUND_FULL_RELAY and MANUAL connections. Unused until next commit. Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2023-08-03Merge bitcoin/bitcoin#28161: ci: Move ASan USDT to persistent_workerfanquake
fa474397b5d4235efdfc5a5ddce2d637234548a7 ci: Add missing linux-headers package to ASan task (MarcoFalke) fabaa85c017467336c7f94ddd83c44935957c919 ci: Move ASan USDT to persistent_worker (MarcoFalke) Pull request description: To run the USDT functional tests, the ASan task currently requires the container host to run the Ubuntu Lunar Linux kernel (or later). Cirrus CI is the only provider that allows to spin up full VMs with Ubuntu Lunar, however they will start to charge for all tasks (See slightly related discussion in https://github.com/bitcoin/bitcoin/issues/28098). Since it is cheaper and recommended by Cirrus CI to just run a persistent worker, do that. Also, using a persistent worker allows to make use of the docker image cache. ACKs for top commit: hebasto: ACK fa474397b5d4235efdfc5a5ddce2d637234548a7, I have reviewed the code and it looks OK. Tree-SHA512: afd084ab1b56cbc3fa44d4611aaa01ec21c1d80aedf1f5f1bc4b8b3d1bd08095e0c7fcea7a3e6ec4b6cd97d01e97ee86061eb84a5e2c7e7195ce02a186254900
2023-08-03Merge bitcoin/bitcoin#27832: doc: Clarify -datacarriersize, add ↵fanquake
-datacarriersize=2 tests faafc35a779745d59fdb0e88698b579215f42b08 doc: Clarify that -datacarriersize applies to the full raw scriptPubKey, not the data push (MarcoFalke) 55550e7fe7e4ffe14637a901b568d1d7e1c716d4 test: Add -datacarriersize=2 tests (MarcoFalke) Pull request description: Clarify with a test that `-datacarriersize` applies to the serialized size of the scriptPubKey, not the size of the pushed data. So for example, * `-datacarriersize=2` will reject a `raw(6a01aa)`, even though only one byte is pushed * `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a)`, even though no byte is pushed * `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a00)`, even though zero bytes are pushed ACKs for top commit: ajtowns: ACK faafc35a779745d59fdb0e88698b579215f42b08 instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/27832/commits/faafc35a779745d59fdb0e88698b579215f42b08 Tree-SHA512: f01ace02798f596ac2a02461e9f2a6ef91b3b37c976ea0b3bc860e2d3efb0ace0fd8b779dd18249cee7f84ebbe5fd21d8506afd3a15edadc00b843ff3b4aacc7
2023-08-03Merge bitcoin/bitcoin#27918: fuzz: addrman, avoid `ConsumeDeserializable` ↵fanquake
when possible 025fda0a76893d09d19ec9a6c0ba86ad11c466f7 fuzz: addrman, avoid `ConsumeDeserializable` when possible (brunoerg) Pull request description: Using specific functions like `ConsumeService`, `ConsumeAddress` and `ConsumeNetAddr` may be more effective than using `ConsumeDeserializable`. They always return some value while `ConsumeDeserializable` may return `std::nullopt`. E.g.: In this part of the code, if `op_net_addr` is `std::nullopt`, we basically generated the addresses (if so) unnecessarily, because we won't be able to use them: ```cpp std::vector<CAddress> addresses; LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) { const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider); if (!opt_address) { break; } addresses.push_back(*opt_address); } const std::optional<CNetAddr> opt_net_addr = ConsumeDeserializable<CNetAddr>(fuzzed_data_provider); if (opt_net_addr) { addr_man.Add(addresses, *opt_net_addr, std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)}); } ``` Also, if we are not calling `Add` effectively, it would also be affect other functions that may "depend" on it. ACKs for top commit: dergoegge: Code review ACK 025fda0a76893d09d19ec9a6c0ba86ad11c466f7 Tree-SHA512: 02450bec0b084c15ba0cd1cbdfbac067c8fea4ccf27be0c86d54e020f029a6c749a16d8e0558f9d6d35a7ca9db8916f180c872f09474702b5591129e9be0d192
2023-08-03scripted-diff: Specify Python major version explicitly on WindowsHennadii Stepanov
Using `py.exe` launcher might by fragile depending on how Python was installed. Specifying the Python version explicitly fixes test errors like this: ``` RunCommandParseJSON error: process(py C:\Users\hebasto\bitcoin\test\functional\mocks\signer.py enumerate) returned 9009: Python was not found... ``` -BEGIN VERIFY SCRIPT- sed -i 's|"py "|"py -3 "|g' $(git grep -l '"py "' -- test/functional) -END VERIFY SCRIPT-
2023-08-03Merge bitcoin/bitcoin#28059: refactor: Make more transaction size variables ↵glozow
signed 92de74ef181b42d774bc6b12329bc0c27caf0081 refactor: Make more transaction size variables signed (Hennadii Stepanov) Pull request description: This PR is a continuation of https://github.com/bitcoin/bitcoin/pull/23962 and it: - gets rid of two static casts, - addresses https://github.com/bitcoin/bitcoin/pull/23962#issuecomment-1593289706, - is useful for https://github.com/bitcoin/bitcoin/pull/25972, see the failed ARM and multiprocess CI jobs. ACKs for top commit: MarcoFalke: lgtm ACK 92de74ef181b42d774bc6b12329bc0c27caf0081 🥔 glozow: ACK 92de74ef181b42d774bc6b12329bc0c27caf0081 Tree-SHA512: 84225961af8e08439664e75661b98fe86560217e891e5633a28316bf248d88df317a0c6b5a5f6b03feb2b0e0fd40a1f91dd4a85a0610d567470805bf47a84487
2023-08-03Merge bitcoin/bitcoin#28204: qa: Close SQLite connection properlyfanquake
703b758e187492d4752270cd9922eaf0af20e2d0 qa: Close SQLite connection properly (Hennadii Stepanov) Pull request description: This PR is a follow-up for https://github.com/bitcoin/bitcoin/pull/26462 that introduced a bug on Windows: ``` >test\functional\wallet_descriptor.py ... PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: ... ``` From `sqlite3` Python module [docs](https://docs.python.org/3/library/sqlite3.html#how-to-use-the-connection-context-manager): > `Connection` object used as context manager only commits or rollbacks transactions, so the connection object should be closed manually. ACKs for top commit: MarcoFalke: lgtm ACK 703b758e187492d4752270cd9922eaf0af20e2d0 theStack: utACK 703b758e187492d4752270cd9922eaf0af20e2d0 Tree-SHA512: 35b1403507be06d1fc04e7e07ff56af5bcfe5013024671f0c1d9f3c41aacc4c777bcc6376ce82d720394e27450415d50ff5d5834ed388ec3f21503f86f1a42a5
2023-08-03refactor: use "if constexpr" in std::vector's Unserialize()Martin Leitner-Ankerl
This gets rid of unnecessarily creating a temporary object T() to call the right function.
2023-08-03refactor: use "if constexpr" in std::vector's Serialize()Martin Leitner-Ankerl
This gets rid of unnecessarily creating a temporary object T() to call the right function.
2023-08-03refactor: use "if constexpr" in prevector's Unserialize()Martin Leitner-Ankerl
This gets rid of unnecessarily creating a temporary object T() to call the right function.
2023-08-03refactor: use "if constexpr" in prevector's Serialize()Martin Leitner-Ankerl
This gets rid of unnecessarily creating a temporary object T() to call the right function.
2023-08-03refactor: use fold expressions instead of recursive calls in UnserializeMany()Martin Leitner-Ankerl
Instead of recursively calling `UnserializeMany` and peeling off one argument at a time, use a fold expression. This simplifies the code, makes it most likely faster because it reduces the number of function calls, and compiles faster because there are fewer template instantiations.
2023-08-03refactor: use fold expressions instead of recursive calls in SerializeMany()Martin Leitner-Ankerl
Instead of recursively calling `SerializeMany` and peeling off one argument at a time, use a fold expression. This simplifies the code, makes it most likely faster because it reduces the number of function calls, and compiles faster because there are fewer template instantiations.
2023-08-02qa: Close SQLite connection properlyHennadii Stepanov
Connection object used as context manager only commits or rollbacks transactions, so the connection object should be closed manually. Fixes the following error on Windows: ``` PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: ... ```
2023-08-02Merge bitcoin/bitcoin#27452: test: cover addrv2 anchors by adding TorV3 to ↵fanquake
CAddress in messages.py ba8ab4fc545800c4fb283a5ff0b19138a0451aba test: cover addrv2 support in anchors.dat with a TorV3 address (Matthew Zipkin) b4bee4bbf45785fbbb355194ccb23c70acd19d27 test: add keep_alive option to socks5 proxy in test_framework (Matthew Zipkin) 5aaf988ccca210228c5a41ea0a18b0c85a85cf71 test: cover TorV3 address in p2p_addrv2_relay (Matthew Zipkin) 80f64a3d40779d342b740fbc57474e170b102678 test: add support for all networks in CAddress in messages.py (brunoerg) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/27140 Adds test coverage for https://github.com/bitcoin/bitcoin/pull/20516 to ensure that https://github.com/bitcoin/bitcoin/issues/20511 is completed and may be closed. This PR adds a test case to `feature_anchors.py` where an onion v3 address is set as a blocks-only relay peer and then shutdown, ensuring that the address is saved to anchors.dat in addrv2 format. We then ensure that bitcoin attempts to reconnect to that anchor address on restart. To compute the addrv2 serialization of the onion v3 address, I added logic to `CAddress` in `messages.py`. This new logic is covered by extending `p2p_addrv2_relay.py` to include an onion v3 address. Future work will be adding coverage for ipv6, torv2 and cjdns in these modules and also `feature_proxy.py` Also includes de/serialization unit test for `CAddress` in test framework. ACKs for top commit: jonatack: ACK ba8ab4fc545800c4fb283a5ff0b19138a0451aba brunoerg: crACK ba8ab4fc545800c4fb283a5ff0b19138a0451aba willcl-ark: ACK ba8ab4fc54 Tree-SHA512: 7220e30d7cb975903d9ac575a7215a08e8f784c24c5741561affcbde12fb92cbf8704cb42e66494b788ba6ed4bb255fb0cc327e4f2190fae50c0ed9f336c0ff0
2023-08-02Merge bitcoin/bitcoin#27572: test: dedup file hashing using `sha256sum_file` ↵fanquake
helper 2c0c6f44770403899bd8514ad7343356853bf38c test: dedup file hashing using `sha256sum_file` helper (Sebastian Falbesoner) Pull request description: Rather than doing the open/read/hash-steps manually in the affected functional tests, we can just use the `sha256sum_file` helper from the utils module instead. Note that for the tool_wallet.py test, the used hash is changed from sha1 to sha256, but as the only purpose is to detect file content changes, this doesn't matter. Also, the optimization using `memoryview` is overkill here, as the opened file has only a size of 24KiB and determining the hash via the helper doesn't take longer than a few hundred micro-seconds on my machine. ACKs for top commit: kristapsk: ACK 2c0c6f44770403899bd8514ad7343356853bf38c Tree-SHA512: 64fe21650b56a50e9f1a95f6ef27d88d8bfbb621e5be456f327ef8dbb5596b529d03976c200f3fd68da48cc427de9f257b403f3228e38cf1df918006674fac68
2023-08-01Merge bitcoin/bitcoin#28144: test: fix intermittent failure in ↵fanquake
p2p_getaddr_caching.py 8a20f765cce2fc0fadf1a2b66b843b67f2d2ae12 test: drop duplicate getaddrs from p2p_getaddr_caching (Martin Zumsande) feb0096139e9e864632d2826d2e213b26146fff1 test: fix intermittent failure in p2p_getaddr_caching (Martin Zumsande) Pull request description: Fixes #28133 In the consistency check, it's not enough to check that our address/port is unique, only the combination of source and target must be unique. Otherwise, the OS may reuse ports for connections to different `-addrbind`, which was happening in the failed runs. While at it, the second commit cleans up duplicate `getaddr` messages in `p2p_getaddr_caching.py` that do nothing but generate `Ignoring repeated "getaddr"` log messages (and cleans up some whitespace the python linter complains about). ACKs for top commit: vasild: ACK 8a20f765cce2fc0fadf1a2b66b843b67f2d2ae12 Tree-SHA512: eabe4727d7887f729074076f6333a918bba8cb34b8e3baaa83f167b441b0daa24f7c4824abcf03a9538a2ef14b2d826ff19aeffcb93a6c20735253a9678aac9c
2023-08-01Merge bitcoin/bitcoin#28166: test, rpc: invalid sighashtype coveragefanquake
90c8f79e945863f3818748b86572948d1558aec3 test: remove redundant test values (Jon Atack) c3f203387df60c596a1e416658d87d68b85adbf2 test: use common assert_signing_completed_successfully helper (Jon Atack) 647d95aae9720543c2c9c46c70191e6f9f775d66 test: add coverage for passing an invalid sighashtype (Jon Atack) Pull request description: Add test coverage for passing an invalid sighashtype to RPCs signrawtransactionwithwallet, signrawtransactionwithkey, walletprocesspsbt, and descriptorprocesspsbt. ACKs for top commit: MarcoFalke: lgtm ACK 90c8f79e945863f3818748b86572948d1558aec3 🎥 brunoerg: light crACK 90c8f79e945863f3818748b86572948d1558aec3 Tree-SHA512: 3861658487edd0d9a377390acf3d43f45c3dd9e324894f0fdb8f5312b618301a55479b1f70c61daee0b20785e768ffde6fa5abe6af190b73c0d0e017f3976704
2023-08-01Merge bitcoin/bitcoin#28060: util: Teach AutoFile how to XORfanquake
fa633aa6906f3b130b691568bcd20b2b76bb1cbb streams: Teach AutoFile how to XOR (MarcoFalke) 000019e158ef01f2bedc3fc1589f95e106e817ea Add AutoFile::detail_fread member function (MarcoFalke) fa7724bc9d94c08d8facccd0a067d6a3b27fbbc6 refactor: Modernize AutoFile (MarcoFalke) fa8d227d58f7baa5a9be1b88930f4813bf6eedb1 doc: Remove comments that just repeat what the code does (MarcoFalke) fafe2ca0ce842cd8f0d8135fa8c8bac9b2c72da6 refactor: Remove redundant file check from AutoFile shift operators (MarcoFalke) 9999a49b3299bd25dde4805f5c68adef3876057f Extract util::Xor, Add key_offset option, Add bench (MarcoFalke) Pull request description: This allows `AutoFile` to roll an XOR pattern while reading or writing to the underlying file. This is needed for https://github.com/bitcoin/bitcoin/pull/28052, but can also be used in any other place. Also, there are tests, so I've split this up from the larger pull to make review easier, hopefully. ACKs for top commit: Crypt-iQ: crACK fa633aa willcl-ark: Lightly tested ACK fa633aa690 jamesob: reACK fa633aa6906f3b130b691568bcd20b2b76bb1cbb ([`jamesob/ackr/28060.4.MarcoFalke.util_add_xorfile`](https://github.com/jamesob/bitcoin/tree/ackr/28060.4.MarcoFalke.util_add_xorfile)) Tree-SHA512: 6d66cad0a089a096d3f95e4f2b28bce80b349d4b76f53d09dc9a0bea4fc1b7c0652724469c37971ba27728c7d46398a4c1d289c252af4c0f83bb2fcbc6f8e90b
2023-08-01ci: Add missing linux-headers package to ASan taskMarcoFalke
Otherwise the task will throw in skip_if_no_python_bcc. Also, adjust CI_CONTAINER_CAP for all needed permissions.
2023-08-01ci: Move ASan USDT to persistent_workerMarcoFalke
2023-08-01Merge bitcoin/bitcoin#28194: test: python E721 and flake8 updatesfanquake
bee2d57a655645dbfaf0242e85c5af034023a2fb script: update flake8 to 6.1.0 (Jon Atack) 38c3fd846bff163eb7c50bd77efcdcf8fcbc7f43 test: python E721 updates (Jon Atack) Pull request description: Update our functional tests per [E721](https://www.flake8rules.com/rules/E721.html) enforced by [flake8 6.1.0](https://flake8.pycqa.org/en/latest/release-notes/6.1.0.html), and update our CI lint task to use that release. This makes the following linter output on current master with flake8 6.1.0 green. ``` $ ./test/lint/lint-python.py ; ./test/lint/lint-spelling.py test/functional/p2p_invalid_locator.py:35:16: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()` test/functional/test_framework/siphash.py:34:12: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()` test/functional/test_framework/siphash.py:64:12: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()` src/test/fuzz/descriptor_parse.cpp:88: occurences ==> occurrences ^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt ``` ACKs for top commit: MarcoFalke: lgtm ACK bee2d57a655645dbfaf0242e85c5af034023a2fb Tree-SHA512: f3788a543ca98e44eeeba1d06c32f1b11eec95d4aef068aa1b6b5c401261adfa3fb6c6d6c769f3fe6839d78e74a310d5c926867e7c367d6513a53d580fd376f3
2023-08-01Merge bitcoin/bitcoin#28131: test: Add UBSan `-fsanitize=integer` ↵fanquake
suppressions for `src/secp256k1` subtree a7477744c5e1df56d3a1e9ab9fc400bfb0ef6ec3 Add UBSan `-fsanitize=integer` suppressions for `src/secp256k1` subtree (Hennadii Stepanov) Pull request description: Required for https://github.com/bitcoin/bitcoin/pull/27991 (see the [comment](https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1611472816)) and for the upcoming CMake-based build system. ACKs for top commit: MarcoFalke: lgtm ACK a7477744c5e1df56d3a1e9ab9fc400bfb0ef6ec3 Tree-SHA512: 602fa3ad22d3b0f6981a51358677d2347c92c4c9f59626b497af10f7ba828ede37227d8ee717f089bf33bde5efe0854d53acc89bea46f0955e62b7f22c454d05
2023-08-01Merge bitcoin/bitcoin#28003: doc: cleanup release process docfanquake
bd5ae6c66317de39195ddb38cea3ca05bbd99275 doc: misc changes in release-process (fanquake) d99ba3cc01360f3d251a1d55c73c501822f83c67 doc: filter out merge-script from list of authors (josibake) 472d6f79b9bf7a714c3672ee88e21483a9a46042 doc: remove generate changelog section from release-process.md (fanquake) 5555ecb80ecc1373bc78b3029d1ee33183a9cdc0 doc: remove note to update bips.md version number (fanquake) Pull request description: Collection of changes to to simplify / correct the release-process documentation. I think we could still simplify this further. For example, we could remove the guix building docs, and defer to `contrib/guix`. ACKs for top commit: MarcoFalke: lgtm ACK bd5ae6c66317de39195ddb38cea3ca05bbd99275 Tree-SHA512: 44bd12dd4f09380daee03fa79f01f9f7e8e2d5cc2fd5ff8c9e85ab54b4ea2b8a35df30ce3bdecfb4ff056cf52822be771ed3419613f68929c122750b1f8c89f9
2023-08-01Merge bitcoin/bitcoin#28070: test: Drop 22.x node from TxindexCompatibilityTestfanquake
fafe43cb6c76a5f60194be128a40baf161d39920 scripted-diff: Use blocks_path where possible (MarcoFalke) fa060c15fb5081e66ed1ebe05dca6e8026f32c4f test: Add blocks_path property to TestNode (MarcoFalke) faba4fc3257c0e7d7bcb5146dab07ffe3193744b test: Drop 22.x node from TxindexCompatibilityTest (MarcoFalke) fa7f65b0f81f3a2899c8eb540182c7d9a90fff00 test: Use clean chain in MempoolCompatibilityTest (MarcoFalke) Pull request description: The node in this test was never really needed, because the compatibility tests shouldn't be used to test previous releases. (The test suite of the previous release itself should be used for that). So remove it. Also, other test changes. (See individual commits) ACKs for top commit: theStack: Code-review ACK fafe43cb6c76a5f60194be128a40baf161d39920 Tree-SHA512: 289f54695bf5310663ab38ebf1aa457f53d0aafae56e6657be0e75bf96b303165bad417dc7eaf4c40f0639aa92ce139e5bacb318a2eabab1f8e23a811cabe0cc
2023-07-31Merge bitcoin/bitcoin#27746: Rework validation logic for assumeutxoRyan Ofsky
a733dd79e29068ad1e0532ac42a45188a040a7b9 Remove unused function `reliesOnAssumedValid` (Suhas Daftuar) d4a11abb1972b54f0babdccfbb2fde97ab885933 Cache block index entry corresponding to assumeutxo snapshot base blockhash (Suhas Daftuar) 3556b850221bc0e597d7dd749d4d47ab58dc8083 Move CheckBlockIndex() from Chainstate to ChainstateManager (Suhas Daftuar) 0ce805b632dcb98944a931f758f76f530f5ce5f2 Documentation improvements for assumeutxo (Ryan Ofsky) 768690b7ce551cd403f8e2a099372915f6022ad4 Fix initialization of setBlockIndexCandidates when working with multiple chainstates (Suhas Daftuar) d43a1f1a2fa35d377c7a9ad7ab92d1ae325bde3d Tighten requirements for adding elements to setBlockIndexCandidates (Suhas Daftuar) d0d40ea9a6478d81d7531b7cfc52a8bdaa0883d6 Move block-storage-related logic to ChainstateManager (Suhas Daftuar) 3cfc75366e6596942cbc84f354f42dfd7fc5c073 test: Clear block index flags when testing snapshots (Suhas Daftuar) 272fbc370c4e133d31d9f1d34e327cc265c5fad2 Update CheckBlockIndex invariants for chains based on an assumeutxo snapshot (Suhas Daftuar) 10c05710ce1602d932037f72dc6c4bbc3f6f34ba Add wrapper for adding entries to a chainstate's block index candidates (Suhas Daftuar) 471da5f6e74bac71aeffe2ebc5faff145a6cbcea Move block-arrival information / preciousblock counters to ChainstateManager (Suhas Daftuar) 1cfc887d00c5d1d4281107e3b3ff4641c6c34631 Remove CChain dependency in node/blockstorage (Suhas Daftuar) fe86a7cd480b32463da900db764d2d11a2bea095 Explicitly track maximum block height stored in undo files (Suhas Daftuar) Pull request description: This PR proposes a clean up of the relationship between block storage and the chainstate objects, by moving the decision of whether to store a block on disk to something that is not chainstate-specific. Philosophically, the decision of whether to store a block on disk is related to validation rules that do not require any UTXO state; for anti-DoS reasons we were using some chainstate-specific heuristics, and those have been reworked here to achieve the proposed separation. This PR also fixes a bug in how a chainstate's `setBlockIndexCandidates` was being initialized; it should always have all the HAVE_DATA block index entries that have more work than the chain tip. During startup, we were not fully populating `setBlockIndexCandidates` in some scenarios involving multiple chainstates. Further, this PR establishes a concept that whenever we have 2 chainstates, that we always know the snapshotted chain's base block and the base block's hash must be an element of our block index. Given that, we can establish a new invariant that the background validation chainstate only needs to consider blocks leading to that snapshotted block entry as potential candidates for its tip. As a followup I would imagine that when writing net_processing logic to download blocks for the background chainstate, that we would use this concept to only download blocks towards the snapshotted entry as well. ACKs for top commit: achow101: ACK a733dd79e29068ad1e0532ac42a45188a040a7b9 jamesob: reACK a733dd79e29068ad1e0532ac42a45188a040a7b9 ([`jamesob/ackr/27746.5.sdaftuar.rework_validation_logic`](https://github.com/jamesob/bitcoin/tree/ackr/27746.5.sdaftuar.rework_validation_logic)) Sjors: Code review ACK a733dd79e29068ad1e0532ac42a45188a040a7b9. ryanofsky: Code review ACK a733dd79e29068ad1e0532ac42a45188a040a7b9. Just suggested changes since the last review. There are various small things that could be followed up on, but I think this is ready for merge. Tree-SHA512: 9ec17746f22b9c27082743ee581b8adceb2bd322fceafa507b428bdcc3ffb8b4c6601fc61cc7bb1161f890c3d38503e8b49474da7b5ab1b1f38bda7aa8668675
2023-07-31script: update flake8 to 6.1.0Jon Atack
and touch up the spelling returned by lint-spelling.py
2023-07-31test: python E721 updatesJon Atack
2023-07-31Merge bitcoin/bitcoin#28091: fuzz: use `ConnmanTestMsg` in `connman`fanquake
ecfe507e07e9bdab210e04ebd3fc3b8ae9d6a094 fuzz: use `ConnmanTestMsg` in `connman` (brunoerg) Pull request description: Fixes #27980 Using `ConnmanTestMsg` we can add nodes and be more effective fuzzing functions like `DisconnectNode`, `FindNode`, `GetNodeStats` and other ones. ACKs for top commit: MarcoFalke: review ACK ecfe507e07e9bdab210e04ebd3fc3b8ae9d6a094 dergoegge: utACK ecfe507e07e9bdab210e04ebd3fc3b8ae9d6a094 Tree-SHA512: 97c363b422809f2e9755c082d1102237347abfab72c7baca417bd8975f8a595ddf3a085f8353dbdb9f17fb98fbfe830792bfc0b83451168458018faf6c239efa
2023-07-31Merge bitcoin/bitcoin#28188: ci: Use documented `CCACHE_MAXSIZE` instead of ↵fanquake
`CCACHE_SIZE` 79ceb161dbf7e033ce557d98e297bc3333665f26 ci: Use documented `CCACHE_MAXSIZE` instead of `CCACHE_SIZE` (Hennadii Stepanov) Pull request description: This PR aims to: 1) Remove our own `CCACHE_SIZE` environment variable that violates Ccache's `CCACHE_*` namespace. 2) Introduce the `CCACHE_MAXSIZE` environment variable that is documented since [v3.3](https://ccache.dev/manual/3.3.html), which makes its usage consistent with other ones, such as `CCACHE_DIR` and `CCACHE_NOHASHDIR`. ACKs for top commit: MarcoFalke: lgtm ACK 79ceb161dbf7e033ce557d98e297bc3333665f26 Tree-SHA512: 13c8a3a3b2337191cab32a3e1c21c19dc465cd4fa9267b2999ca2fde5cca0c03811f520cd3940959aa57ea9cf0251db325df56a90fca85069d04ce2141ec7044
2023-07-31Merge bitcoin/bitcoin#28181: qa, doc: Fix commentfanquake
ab498d913c6f9f6096c75cc43a91e7a12cfc3fb7 qa, doc: Fix comment (Hennadii Stepanov) Pull request description: This PR is a follow-up for: - https://github.com/bitcoin/bitcoin/pull/9956 - https://github.com/bitcoin/bitcoin/pull/10096 ACKs for top commit: RandyMcMillan: ACK ab498d913c6f9f6096c75cc43a91e7a12cfc3fb7 Tree-SHA512: 267ae52c961ee79e172f27cb1587282ac5cf7ec929a136db6feed3de4f319a74b462befd9b99711081db8a5c30a513f72366364068700418b273a31958b31b1d
2023-07-30ci: Use documented `CCACHE_MAXSIZE` instead of `CCACHE_SIZE`Hennadii Stepanov
This change aims to: 1) Remove our own `CCACHE_SIZE` environment variable that violates Ccache's `CCACHE_*` namespace. 2) Introduce the `CCACHE_MAXSIZE` environment variable that is documented since v3.3, which makes its usage consistent with other ones, such as `CCACHE_DIR` and `CCACHE_NOHASHDIR`.
2023-07-30Merge bitcoin/bitcoin#28118: test: Add SyncWithValidationInterfaceQueue to ↵fanquake
mockscheduler RPC fabef121b0cdfac6ec1985f6c08c5685a886ba5a refactor: Use EnsureAnyNodeContext (MarcoFalke) fa1640617e061431059908fbf496dccca6b4e112 test: Add SyncWithValidationInterfaceQueue to mockscheduler RPC (MarcoFalke) Pull request description: There should be no risk or downside in adding a call to `SyncWithValidationInterfaceQueue` here. In fact, it will make tests less brittle. For example, * If one sets the timeouts in `test/functional/feature_fee_estimation.py` to `0`, on `master` the test will fail and here it will pass. * It may avoid a rare (theoretic) intermittent issue in https://github.com/bitcoin/bitcoin/pull/28108/files#r1268966663 ACKs for top commit: TheCharlatan: ACK fabef121b0cdfac6ec1985f6c08c5685a886ba5a furszy: Code review ACK fabef121. Convinced by checking all current tests usages. Tree-SHA512: c9e9a536a8721d1b3f267a66b40578b34948892301affdcad121ef8e02bf17037305d0dd53aa94b1b064753e66f9cfb31823b916b707a9d812627f502b818003
2023-07-30qa, doc: Fix commentHennadii Stepanov
This change is a follow-up for: - https://github.com/bitcoin/bitcoin/pull/9956 - https://github.com/bitcoin/bitcoin/pull/10096
2023-07-28Merge bitcoin/bitcoin#28138: ci: Keep system env vars as-is (bugfix)fanquake
fabc04a4d96c4fe70e60d365aa28031d149094f3 ci: Keep system env vars as-is (MarcoFalke) fa8dcdcc8b29e58f5d285a49dde33d94b63c893b ci: Set PATH inside the CI env (MarcoFalke) fac229ab1f95ec77f18be8a783a2779dd781c684 ci: Remove P_CI_DIR and --workdir (MarcoFalke) Pull request description: This fixes a bug where the `$PATH` from the host is used inside the container. This will lead to bugs when the `$PATH` is different. For example on a host of Fedora 38, and a container of `debian:bullseye`. This can be tested with the `FILE_ENV=./ci/test/00_setup_env_arm.sh` CI env. On master: ``` Error: crun: executable file `bash` not found in $PATH: No such file or directory: OCI runtime attempted to invoke a command that was not found ``` On this pull: (everything passes) ACKs for top commit: TheCharlatan: lgtm ACK fabc04a4d96c4fe70e60d365aa28031d149094f3 Tree-SHA512: 51d2affed91624d0e5b43a3ee1e696313f934e05fde6b5a19e8ac4e6666c1e7b667a444bf3de3f77190bcd00e81efb7576196afb0f6b5e788d1a50e7ddb28d7e
2023-07-28Merge bitcoin/bitcoin#28162: refactor: Revert additional univalue check in ↵fanquake
ParseSighashString 06199a995f20c55583f6948cfe99e608679fcdf1 refactor: Revert addition of univalue sighash string check (TheCharlatan) 0b47c1621524a96b79cbdc3c45ee5ad36e7ba541 doc: Correct release-notes for sighashtype exceptions (TheCharlatan) Pull request description: This is a follow up for #28113. The string type check is already done by the rpc parser / RPCHelpMan. Re-doing it is adding dead code. Instead, throwing an exception when the assumption does not hold is the already correct behavior. Pointed out in this [comment](https://github.com/bitcoin/bitcoin/pull/28113/files#r1274568557). Also correct the release note for the correct sighashtype exception change. There is no change in the handling of non-string sighashtype arugments. Pointed out in this [comment](https://github.com/bitcoin/bitcoin/pull/28113/files#r1274567555). ACKs for top commit: MarcoFalke: lgtm ACK 06199a995f20c55583f6948cfe99e608679fcdf1 jonatack: Tested ACK 06199a995f20c55583f6948cfe99e608679fcdf1 stickies-v: ACK 06199a995f20c55583f6948cfe99e608679fcdf1 Tree-SHA512: 3faa6b3d2247624c0973df8d79c09fbf1f90ffb99f1be484e359b528f485c31affea45976759bd206e4c81cbb54ebba5ad0ef4127d1deacbfe2a58153fcc94ee
2023-07-28Merge bitcoin/bitcoin#28168: refactor: Remove unused raw-pointer read helper ↵fanquake
from univalue fa940f41eaffa4b2a28c465a10a4c12d4b8976b8 Remove unused raw-pointer read helper from univalue (MarcoFalke) Pull request description: The helpers are unused outside of tests and redundant with the existing `bool read(std::string_view raw);`. Fix both issues by removing them. Also, simplify the tests code by removing a `std::string` constructor where possible. ACKs for top commit: stickies-v: utACK fa940f41eaffa4b2a28c465a10a4c12d4b8976b8 TheCharlatan: tACK fa940f41eaffa4b2a28c465a10a4c12d4b8976b8 Tree-SHA512: 60c154c1046f01551335af79bf820a6104844f63e89977271b4336b3cd06ac3bab1379e18b7bc61d12bef7446029e91c16541ddecf9e88bc8bc897fc1f6ee2c8
2023-07-27Merge bitcoin/bitcoin#27888: Fuzz: a more efficient descriptor parsing targetAndrew Chow
131314b62e899f95d1863083d303b489b3212b16 fuzz: increase coverage of the descriptor targets (Antoine Poinsot) 90a24741e79cbf20d4456050f0fe39c3f88f5246 fuzz: add a new, more efficient, descriptor parsing target (Antoine Poinsot) d60229ede54e05724d444eaba02a9ed72f5ada02 fuzz: make the parsed descriptor testing into a function (Antoine Poinsot) Pull request description: The current descriptor parsing fuzz target requires valid public or private keys to be provided. This is unnecessary as we are only interested in fuzzing the descriptor parsing logic here (other targets are focused on fuzzing keys serializations). And it's pretty inefficient, especially for formats that need a checksum (`xpub`, `xprv`, WIF). This introduces a new target that mocks the keys as an index in a list of precomputed keys. Keys are represented as 2 hex characters in the descriptor. The key type (private, public, extended, ..) is deterministically based on this one-byte value. Keys are deterministically generated at target initialization. This is much more efficient and also largely reduces the size of the seeds. TL;DR: for instance instead of requiring the fuzzer to generate a `pk(xpub6DdBu7pBoyf7RjnUVhg8y6LFCfca2QAGJ39FcsgXM52Pg7eejUHLBJn4gNMey5dacyt4AjvKzdTQiuLfRdK8rSzyqZPJmNAcYZ9kVVEz4kj)` to parse a valid descriptor, it just needs to generate a `pk(03)`. Note we only mock the keys themselves, not the entire descriptor key expression. As we want to fuzz the real code that parses the rest of the key expression (origin, derivation paths, ..). This is a target i used for reviewing #17190 and #27255, and figured it was worth PR'ing on its own since the added complexity for mocking the keys is minimal and it could help prevent introducing bugs to the descriptor parsing logic much more efficiently. ACKs for top commit: MarcoFalke: re-ACK 131314b62e899f95d1863083d303b489b3212b16 🐓 achow101: ACK 131314b62e899f95d1863083d303b489b3212b16 Tree-SHA512: 485a8d6a0f31a3a132df94dc57f97bdd81583d63507510debaac6a41dbbb42fa83c704ff3f2bd0b78c8673c583157c9a3efd79410e5e79511859e1470e629118
2023-07-27Merge bitcoin/bitcoin#28148: refactor: consistently use ApplyArgsManOptions ↵Andrew Chow
for PeerManager::Options 8a3159728ae84cb8093e2e9fa5d2c2b0a7d545da refactor: deduplicate ignores_incoming_txs (stickies-v) 5f41afcc46913dbd4b5f08e622c5f74cd1eb50a5 refactor: set ignore_incoming_txs in ApplyArgsManOptions (stickies-v) Pull request description: Consistently use `ApplyArgsManOptions` for `PeerManager::Options`, and initialize `PeerManager::Options` early to avoid reading `"-blocksonly"` twice. Suggested in https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268400386 and also requested in https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1273346189. No behaviour change, but the [`TestingSetup`](https://github.com/bitcoin/bitcoin/blob/e35fb7bc48d360585b80d0c7f89ac5087c1d405e/src/test/util/setup_common.cpp#L255-L256) is now also able to access `"-blocksonly"`. ACKs for top commit: MarcoFalke: lgtm ACK 8a3159728ae84cb8093e2e9fa5d2c2b0a7d545da achow101: ACK 8a3159728ae84cb8093e2e9fa5d2c2b0a7d545da TheCharlatan: ACK 8a3159728ae84cb8093e2e9fa5d2c2b0a7d545da dergoegge: utACK 8a3159728ae84cb8093e2e9fa5d2c2b0a7d545da Tree-SHA512: 6cb489d79ac2a87e8faedb76c96973ab3fc597426f274a90a3ffd0bc5fe3f2b25db9c7ec2e55a0c806c2bcbc0fdded6e228adb43d2cd81f14fd6552863847698
2023-07-27test: remove redundant test valuesJon Atack
as they are parsed identically. See AmountFromValue() / ParseFixedPoint() / UniValue#getValStr()
2023-07-27test: use common assert_signing_completed_successfully helperJon Atack
2023-07-27test: add coverage for passing an invalid sighashtypeJon Atack
in RPCs descriptorprocesspbst, walletprocesspbst, signrawtransactionwithkey, and signrawtransactionwithwallet.
2023-07-27Remove unused raw-pointer read helper from univalueMarcoFalke
2023-07-27Merge bitcoin/bitcoin#28164: test: remove unused code in ↵fanquake
`wallet_fundrawtransaction` 108c6255bc670bbf2732f2b79f6c32c26e181208 test: remove unused `totalOut` code (brunoerg) 0fc3deee9a34d2f8e8014da776e6cefc2bd6f664 test: remove unecessary `decoderawtransaction` calls (brunoerg) Pull request description: This PR removes in `wallet_fundrawtransaction`: - unecessary variables/calls to `decoderawtransaction` - unused `totalOut` variable and its related code (`totalOut` is used in some functions to test change, in other ones its value is not used) ACKs for top commit: kevkevinpal: utACK [108c625](https://github.com/bitcoin/bitcoin/pull/28164/commits/108c6255bc670bbf2732f2b79f6c32c26e181208) MarcoFalke: lgtm ACK 108c6255bc670bbf2732f2b79f6c32c26e181208 Tree-SHA512: c352524f3633146117534c79bd1a24523a7068f13a17d0b8a425cc3c85d62cb769a79ea60db8b075b137da2a0cc43142c43a23ca5af89246ff86cd824e37cf17