82bb7831fa6052620998c7eef47e48ed594248a8 wallet: skip block scan if block was created before wallet birthday (furszy)
a082434d122754ec1a10e0e08a35bdb1f47989e6 refactor: single method to append new spkm to the wallet (furszy)
Pull request description:
During initial block download, the node's wallet(s) scans every arriving block looking for data that it owns.
This process can be resource-intensive, as it involves sequentially scanning all transactions within each
arriving block.
To avoid wasting processing power, we can skip blocks that occurred before the wallet's creation time,
since these blocks are guaranteed not to contain any relevant wallet data.
This has direct implications (an speed improvement) on the underlying blockchain synchronization process
as well. The reason is that the validation interface queue is limited to 10 tasks per time. This means that no
more than 10 blocks can be waiting for the wallet(s) to be processed while we are synchronizing the chain
(activating the best chain to be more precise).
Which can be a bottleneck if blocks arrive and are processed faster from the network than what they are
processed by the wallet(s).
So, by skipping not relevant blocks in the wallet's IBD scanning process, we will also improve the chain
synchronization time.
ACKs for top commit:
ishaanam:
re-ACK 82bb7831fa6052620998c7eef47e48ed594248a8
achow101:
re-ACK 82bb7831fa6052620998c7eef47e48ed594248a8
pinheadmz:
ACK 82bb7831fa6052620998c7eef47e48ed594248a8
Tree-SHA512: 70158c9657f1fcc396badad2c4410b7b7f439466142640b31a9b1a8cea4555e45ea254e48043c9b27f783d5e4d24d91855f0d79d42f0484b8aa83cdbf3d6c50b
fb02a3cd1a105bdf60ca39e1858e77685be88976 p2p: Log addresses of stalling peers (Martin Zumsande)
Pull request description:
This was suggested in #27705 by ArmchairCryptologist.
It allows node operators that have the `-logips` option enabled to better identify potentially misbehaving peers and maybe ban them.
This is especially helpful in case of inbound peers for which (dis)connections aren't logged per default, so it's impossible to use the debug log to connect their `nodeId` to an address unless the very noisy `net` debugging is enabled.
In case of outbound peers for which the address is potentially logged when establishing the connection, this just adds some convenience.
ACKs for top commit:
stratospher:
tACK fb02a3c.
jamesob:
github ACK fb02a3cd1a
0xB10C:
Untested ACK fb02a3cd1a105bdf60ca39e1858e77685be88976
instagibbs:
utACK fb02a3cd1a105bdf60ca39e1858e77685be88976
Tree-SHA512: 2080f794c715bd36143405828b4b0e1be859095caf8f8a0c20dd2a4b64d192d78fee0fa350a2bb7c39848718332c4dd4d8edb2cc8d22095b65afe710591f7ccb
ffffe622e9cbf926326135bb23958380dcf09df1 test: Move test_chain_listunspent wallet check from mempool_packages to wallet_basic (MarcoFalke)
Pull request description:
This fixes a bug.
On master:
```
$ ./test/functional/mempool_packages.py --legacy-wallet
File "./test/functional/mempool_packages.py", line 52, in run_test
self.nodes[0].importaddress(self.wallet.get_address())
test_framework.authproxy.JSONRPCException: Bech32m addresses cannot be imported into legacy wallets (-5)
```
On this pull, all tests pass.
ACKs for top commit:
glozow:
ACK ffffe622e9cbf926326135bb23958380dcf09df1, thanks for changing! Nice to remove wallet from another non-wallet test.
Tree-SHA512: 842c3b7c2e90285a155b8ed9924ef0c99f7773892be4f1847e5d7ece79c914ea5acee0d71de2ce46c354ee95fb74a03c20c0afb5e49c0b8e1c0ce406df963650
eefe56967b4eb4b5144325cde4f40fc1cbde3e65 bugfix: Fix incorrect debug.log config file path (Ryan Ofsky)
3746f00be1b732a04976fc70cbb0661f97bbbd99 init: Error if ignored bitcoin.conf file is found (Ryan Ofsky)
398c3719b02197ad92fded20f6ff83b364747297 lint: Fix lint-format-strings false positives when format specifiers have argument positions (Ryan Ofsky)
Pull request description:
Show an error on startup if a bitcoin datadir that is being used contains a `bitcoin.conf` file that is ignored. There are two cases where this could happen:
- One case reported in [#27246 (comment)](https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043) happens when a `bitcoin.conf` file in the default datadir (e.g. `$HOME/.bitcoin/bitcoin.conf`) has a `datadir=/path` line that sets different datadir containing a second `bitcoin.conf` file. Currently the second `bitcoin.conf` file is ignored with no warning.
- Another way this could happen is if a `-conf=` command line argument points to a configuration file with a `datadir=/path` line and that path contains a `bitcoin.conf` file, which is currently ignored.
This change only adds an error message and doesn't change anything about way settings are applied. It also doesn't trigger errors if there are redundant `-datadir` or `-conf` settings pointing at the same configuration file, only if they are pointing at different files and one file is being ignored.
ACKs for top commit:
pinheadmz:
re-ACK eefe56967b4eb4b5144325cde4f40fc1cbde3e65
willcl-ark:
re-ACK eefe56967b
TheCharlatan:
ACK eefe56967b4eb4b5144325cde4f40fc1cbde3e65
Tree-SHA512: 939a98a4b271b5263d64a2df3054c56fcde94784edf6f010d78693a371c38aa03138ae9cebb026b6164bbd898d8fd0845a61a454fd996e328fd7bcf51c580c2b
4fe5f3c4675263ea106e7ac6d336ec769392ebc3 depends: remove redundant stdlib option (Cory Fields)
Pull request description:
Like #27628, this is another dependency of #21778, though it doesn't become obvious until used with a newer clang.
This should be a no-op.
Use of -stdlib++-isystem gets rid of any system c++ header include paths and negates the need for this option. In newer versions of clangs the combo produces an annoying warning that actually causes problems during configure.
ACKs for top commit:
fanquake:
ACK 4fe5f3c4675263ea106e7ac6d336ec769392ebc3
Tree-SHA512: 904072f2b13dffbbeab2bc9ff20a74969888502fa1ea35d9030784dd397c6751e31233e6ec7dc34e9fd42574950be733b25d6653c2a93e214cc5e4eef2e0133a
To avoid wasting processing power, we can skip blocks that occurred
before the wallet's creation time, since these blocks are guaranteed
not to contain any relevant wallet data.
This has direct implications (an speed improvement) on the underlying
blockchain synchronization process as well.
The reason is that the validation interface queue is limited to
10 tasks per time. This means that no more than 10 blocks can be
waiting for the wallet(s) to be processed while we are synchronizing
the chain (activating the best chain to be more precise).
Which can be a bottleneck if blocks arrive and are processed faster
from the network than what they are processed by the wallet(s).
3d0a5c37e9ccedfa4ecfaa48eeeca1ada5b4eec1 use 'byte'/'bytes' for bech32(m) validation error (Reese Russell)
Pull request description:
This PR rectifies a linguistic inconsistency found in merged PR [27727](https://github.com/bitcoin/bitcoin/pull/27727). It addresses the improper usage of the term 'byte' in error reports. As it stands, PR [27727](https://github.com/bitcoin/bitcoin/pull/27727) exclusively utilizes 'byte' in error messages, regardless of the context, as demonstrated below:
Currently: ```Invalid Bech32 v0 address program size (16 byte), per BIP141```
This modification enhances the accuracy of error reporting in most scenarios users are likely to encounter by checking for a plural or singular number of bytes.
This PR
**16 Bytes program size error** :
```
(
"BC1QR508D6QEJXTDG4Y5R3ZARVARYV98GJ9P",
"Invalid Bech32 v0 address program size (16 bytes), per BIP141",
[],
)
```
**1 Byte program size error**
```
(
"bc1pw5dgrnzv",
"Invalid Bech32 address program size (1 byte)",
[]
),
```
Thank you
ACKs for top commit:
MarcoFalke:
lgtm ACK 3d0a5c37e9ccedfa4ecfaa48eeeca1ada5b4eec1
Tree-SHA512: 55069194a6a33a37559cf14b59b6ac05b1160f57f14d1415aef8e76c916c7c7f343310916ae85b3fa895937802449c1dddb2f653340d0f39203f06aee10f6fce
d97269579769effbe6eec2303ea0cc3e396d3e0d Unconditionally return when compact block status == READ_STATUS_FAILED (Greg Sanders)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204491894 which would have resulted in wasted bandwidth every once in a while.
ACKs for top commit:
Sjors:
utACK d97269579769effbe6eec2303ea0cc3e396d3e0d
mzumsande:
ACK d97269579769effbe6eec2303ea0cc3e396d3e0d
Tree-SHA512: 2483cae03093fc61b36279bbc455829822258703ae608ff2797a20449410b12bfe8d40de63c02fc79ed4ef4d91b34c63281b71b81a1c691cab9647b65761586f
eeee55f9288740747b6e8d806ce8177fd92635cf rpc: Fix invalid bech32 handling (MarcoFalke)
Pull request description:
Currently the handling of invalid bech32(m) addresses over RPC has many issues:
* No error for invalid addresses is reported, leading to internal bugs via `CHECK_NONFATAL`, see https://github.com/bitcoin/bitcoin/issues/27723
* The error messages use "data size" (the meaning of which is unclear to the user, because the witness program data and bech32 section data are related but different) when they mean "program size"
Fix all issues. Also, use the BIP 173 and BIP 350 test vectors.
ACKs for top commit:
achow101:
ACK eeee55f9288740747b6e8d806ce8177fd92635cf
brunoerg:
crACK eeee55f9288740747b6e8d806ce8177fd92635cf
Tree-SHA512: c8639ee49e2a54b740b72d66bc4a40352dd553a6e3220dea9f94e48e33124f21f597a2817cb405d0a4c88d21df1013c0a4877a01370a2d326aa2cff1f9c381a8
d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83 Add tests for parallel compact block downloads (Greg Sanders)
03423f8bd12b95a06a4a9d8377e781625dd38aae Support up to 3 parallel compact block txn fetchings (Greg Sanders)
13f9b20b4cb2f3f26e81184a77e9cf1f626d4f57 Only request full blocks from the peer we thought had the block in-flight (Greg Sanders)
cce96182ba2457335868c65dc16b081c3dee32ee Convert mapBlocksInFlight to a multimap (Greg Sanders)
a90595478dcf4e443cd15bbb822d485dc42bdb18 Remove nBlocksInFlight (Greg Sanders)
86cff8bf18f2c6344a25ad8b81cf366201a73c36 alias BlockDownloadMap for mapBlocksInFlight (Greg Sanders)
Pull request description:
This is an attempt at mitigating https://github.com/bitcoin/bitcoin/issues/25258 , which is a revival of https://github.com/bitcoin/bitcoin/pull/10984, which is a revival of https://github.com/bitcoin/bitcoin/pull/9447.
This PR attempts to mitigate a single case, where high bandwidth peers can bail us out of a flakey
peer not completing blocks for us. We allow up to 2 additional getblocktxns requests per unique block.
This would hopefully allow the chance for an honest high bandwidth peer to hand us the transactions
even if the first in flight peer stalls out.
In contrast to previous effort:
1) it will not help if subsequent peers send block headers only, so only high-bandwidth peers this time. See: https://github.com/bitcoin/bitcoin/pull/10984/files#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3R1411
2) `MAX_GETBLOCKTXN_TXN_AFTER_FIRST_IN_FLIGHT` is removed, in favor of aiding recovery during turbulent mempools
3) We require one of the 3 block fetching slots to be an outbound peer. This can be the original offering peer, or subsequent compact blocks given by high bandwidth peers.
ACKs for top commit:
sdaftuar:
ACK d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83
mzumsande:
Code Review ACK d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83
Tree-SHA512: 54980eac179e30f12a0bd49df147b2c3d63cd8f9401abb23c7baf02f76eeb59f2cfaaa155227990d0d39384de9fa38663f88774e891600a3837ae927f04f0db3
272eb5561667482f8226bcf98eea00689dccefb8 test: fix `include_immature_coinbase` logic in `get_utxos` (brunoerg)
a951c34f179dad0c7059b04a7f1e6b0804462168 test: fix `interface_usdt_mempool` by mining a block after each test (brunoerg)
1557bf1196bc2bdf00fd32f3b5d525796b4d194c test: fix mature utxos addition to wallet in `mempool_package_limits` (brunoerg)
60ced9007d518d542ce489b91076f9bbaf3312e3 test: fix intermittent issue in `feature_bip68_sequence` (brunoerg)
Pull request description:
Fixes#27129
To avoid `bad-txns-premature-spend-of-coinbase` error,
when getting a utxo (using `get_utxo`) to create a new
transaction `get_utxo` shouldn't return (if possible)
by default immature coinbase.
ACKs for top commit:
achow101:
ACK 272eb5561667482f8226bcf98eea00689dccefb8
pinheadmz:
re-ACK 272eb5561667482f8226bcf98eea00689dccefb8
Tree-SHA512: eae821c7833bf084d8b907c94876ed010a7925d2177c3013a0c61b69d9571df006da83397a19487d93b0d1fa415951152f0b8ad0de2a55d86c39f6917934f050
A single outbound slot is required, so if the first two slots
are taken by inbound in-flights, the node will reject additional
unless they are coming from outbound.
This means in the case where a fast sybil peer is attempting to
stall out a node, a single high bandwidth outbound peer can
mitigate the attack.
4f2f615d1362afe92cabe9eab50087f8bfe454fd test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX` (Hennadii Stepanov)
Pull request description:
This PR is a continuation of changes to our testing frameworks (https://github.com/bitcoin/bitcoin/pull/27554, https://github.com/bitcoin/bitcoin/pull/27561) that allow them to work correctly in a multi-config build environment that is possible for [upcoming](https://github.com/bitcoin/bitcoin/pull/25797) CMake-based build system. That means that built for different configurations binaries (e.g., "Debug" and "Release") can coexist in separated directories.
The commit has been pulled from https://github.com/hebasto/bitcoin/pull/15 and it seems [useful](https://github.com/hebasto/bitcoin/pull/15#discussion_r1200251404) by itself as:
> I believe the rationale for allowing to drop in the executables via env var is to allow to test the guix-produced, or other third-party-produced executables...
The current implementation of the `test/functional/test_framework/test_framework.py` script uses the same approach: 09351f51d2/test/functional/test_framework/test_framework.py (L231-L246)
ACKs for top commit:
MarcoFalke:
lgtm ACK 4f2f615d1362afe92cabe9eab50087f8bfe454fd
TheCharlatan:
ACK 4f2f615d1362afe92cabe9eab50087f8bfe454fd
stickies-v:
ACK 4f2f615d1362afe92cabe9eab50087f8bfe454fd
Tree-SHA512: 99ee9a727b266700649d8f2ec528dfaeb04a1e48f0cb1d4eeaece65917165be647c10c4548429a9e8b30d094597f67e860c1db03ac689ebb409b223ce1b63aa9
Use of -stdlib++-isystem gets rid of any system c++ header include paths and
negates the need for this option. In newer versions of clangs the combo
produces a warning.
1bce12acd3e271a7c88d9400b4e3a5645bc8a911 test: add test for `descriptorprocesspsbt` RPC (ishaanam)
fb2a3a70e860aa87fb7a21f6554ed9f3ce901e2d rpc: add descriptorprocesspsbt rpc (ishaanam)
Pull request description:
This PR implements an RPC called `descriptorprocesspsbt`. This RPC is based off of `walletprocesspsbt`, but instead of interacting with the wallet to update, sign and finalize a psbt, it instead accepts an array of output descriptors and uses that information along with information from the mempool, txindex, and the utxo set to do so. `utxoupdatepsbt` also updates a psbt in this manner, but doesn't sign or finalize it. Because of this overlap, a helper function that is added in this PR is called by both `utxoupdatepsbt` and `descriptorprocesspsbt`. Whether or not the helper function signs a psbt is dictated by if the HidingSigningProvider passed to it contains any private information. There is also a test added in this PR for this new RPC that uses p2wsh, p2wpkh, and legacy outputs.
Edit: see https://github.com/bitcoin/bitcoin/pull/25796#issuecomment-1228830963
ACKs for top commit:
achow101:
re-ACK 1bce12acd3e271a7c88d9400b4e3a5645bc8a911
instagibbs:
reACK 1bce12acd3
Tree-SHA512: e1d0334739943e71f2ee68b4db7637ebe725da62e7aa4be071f71c7196d2a5970a31ece96d91e372d34454cde8509e95ab0eebd2c8edb94f7d5a781a84f8fc5d
fa1b3abc834a90fb1cfbd5ac63deb28f3990c1fb ci: Log qa-assets repo last commit (MarcoFalke)
fa22966f3307e22b77ea386e1abb60bf8c606170 fuzz: Print error message when FUZZ is missing (MarcoFalke)
Pull request description:
Some trivial UX improvements.
* Change the exit code for `PRINT_ALL_FUZZ_TARGETS_AND_ABORT` and `WRITE_ALL_FUZZ_TARGETS_AND_ABORT` to `EXIT_SUCCESS` instead of `Aborted (core dumped)`.
* Print readable error message when `FUZZ` is missing instead of `Aborted (core dumped)`.
* Clarify that a fuzz target needs to be compiled into the executable.
ACKs for top commit:
dergoegge:
ACK fa1b3abc834a90fb1cfbd5ac63deb28f3990c1fb
Tree-SHA512: 065ef8920449c64b3516f89a61cb397b505eccf531318c4f3830895d5ff6cd7ae2525cb857320481e3d0ed0b2f8a522cd8f7835e69f021241b6ec297a6102fc8
5228223e1ff2af29e6e77668ce3288005c2adbbc ci: remove MSAN getrandom syscall workaround (fanquake)
d5e06919db5e221bfef445c5a40c88de72dc5869 random: switch to using getrandom() directly (fanquake)
c2ba3f5b0c7d0eece7d16d1ffc125d8a6a9297af random: add [[maybe_unused]] to GetDevURandom (fanquake)
c13c97dbf846cf0e6a5581ac414ef96a215b0dc6 random: getentropy on macOS does not need unistd.h (fanquake)
Pull request description:
This requires a linux kernel of `3.17`+, which seems entirely
reasonable. `3.17` went EOL in 2015, and the last supported `3.x` kernel
(`3.16`) went EOL > 4 years ago, in 2020. For reference, the current
oldest maintained kernel is `4.14` (released 2017, going EOL Jan 2024).
Support for `getrandom()` (and `getentropy()`) was added to
glibc `2.25` https://sourceware.org/legacy-ml/libc-alpha/2017-02/msg00079.html:
> * The getentropy and getrandom functions, and the <sys/random.h> header
file have been added.
and we already require `2.27` or later.
All that being said, I don't think you would encounter a current day (+~6 months from now)
system, running with kernel headers older than 3.17 (released 2014) but also having a
glibc of 2.27+ (released 2018)?
Removing this (our only) use of `syscall()` also means we can drop a workaround in our MSAN jobs.
If this is merged, I'll drop the [same workaround in oss-fuzz](25946a5448/projects/bitcoin-core/build.sh (L49-L56)).
ACKs for top commit:
josibake:
ACK 5228223e1f
hebasto:
ACK 5228223e1ff2af29e6e77668ce3288005c2adbbc, I've tested build system changes on Ubuntu 22.04 and macOS Monterey 12.6.6 (x86_64).
Tree-SHA512: cc978e08510c461b875ca8c08ae176b4519fa1108f0efd74dcb7474518945357e0184e54423282c9a496de195e4ddc3e221ee78623bd63e24c50cc86acdf32e2
c44f3f231988dc05c4c7a8a96bc2e7b1a54da277 test: Explicitly specify directory where to search tests for (Hennadii Stepanov)
Pull request description:
For out-of-source builds, the `test/functional/test_runner.py` is supposed to be run from the build directory which allows it to pick the `test/config.ini` file generated by the build system. Currently, it works accidently for the following reasons:
- on POSIX systems, when running a created by Autoconf symlink to the `test/functional/test_runner.py` in the source directory, it actually has the source directory location in the `sys.path`.
- on Windows (the `build_msvc` directory) VS project puts and copies every build artifact into the source tree (which is wrong and ugly).
This PR makes `test/functional/test_runner.py` work from a build directory in any form (a symbolic link, a hard link, a copy) on _all_ supported platforms, which is highly desirable in the upcoming [CMake-based build system](https://github.com/bitcoin/bitcoin/pull/25797).
For the current master branch, this PR has no behaviour change.
Required for https://github.com/hebasto/bitcoin/pull/15.
---
**Steps to reproduce the issue**
While the issue is mostly specific to Windows and CMake builds, it is still possible to reproduce it on the current master branch.
1. Make an out-of-source build:
```
$ ./autogen.sh
$ mkdir ../build && cd ../build
$ ../bitcoin/configure
$ make
```
2. Note that Autoconf created a symbolic link `test/functional/test_runner.py` in the `../build` directory:
```
$ ls -l test/functional/test_runner.py
lrwxrwxrwx 1 hebasto hebasto 47 May 5 17:40 test/functional/test_runner.py -> ../../../bitcoin/test/functional/test_runner.py
```
which works flawlessly.
3. However, replacing this symbolic link with a hard link or a copy of `test/functional/test_runner.py` from the source tree will cause the following error:
```
$ cp ../bitcoin/test/functional/test_runner.py test/functional/test_runner.py
$ ls -l test/functional/test_runner.py
$ ./test/functional/test_runner.py
Temporary test directory at /tmp/test_runner_₿_🏃_20230505_175104
Running Unit Tests for Test Framework Modules
E
======================================================================
ERROR: test_framework (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_framework
Traceback (most recent call last):
File "/usr/lib/python3.10/unittest/loader.py", line 154, in loadTestsFromName
module = __import__(module_name)
ModuleNotFoundError: No module named 'test_framework'
----------------------------------------------------------------------
Ran 1 test in 0.000s
FAILED (errors=1)
Early exiting after failure in TestFramework unit tests
```
ACKs for top commit:
stickies-v:
re-ACK c44f3f2319
MarcoFalke:
lgtm ACK c44f3f231988dc05c4c7a8a96bc2e7b1a54da277 💸
Tree-SHA512: 622ff629080a55f76dd4c1dab6699de0e9f06b75da3315cd3b31b972ef4bde746458bf3e8a95e879b3c6a63be2368af70005a83f6a3c85c4f1ba5be51e91a61d
fa5831bd6f940c4afb43ff625ba4fa6c641e999a build: Do not define `ENABLE_ZMQ` when ZMQ is not available (Hennadii Stepanov)
Pull request description:
A new behavior is consistent with the other optional dependencies.
The source code contains `#if ENABLE_ZMQ` lines only:
```
$ git grep ENABLE_ZMQ -- src/*.cpp
src/init.cpp:#if ENABLE_ZMQ
src/init.cpp:#if ENABLE_ZMQ
src/init.cpp:#if ENABLE_ZMQ
src/init.cpp:#if ENABLE_ZMQ
src/init.cpp:#if ENABLE_ZMQ
```
Change in description line -- "Define to 1..." --> "Define this symbol.." -- is motivated by the fact that the actual value of the defined `ENABLE_ZMQ` macro does not matter at all.
Related to:
- https://github.com/bitcoin/bitcoin/issues/16419
- https://github.com/bitcoin/bitcoin/pull/25302
ACKs for top commit:
TheCharlatan:
ACK fa5831bd6f940c4afb43ff625ba4fa6c641e999a
jarolrod:
ACK fa5831bd6f940c4afb43ff625ba4fa6c641e999a
Tree-SHA512: 5e72ff0d34c4b33205338daea0aae8d7aa0e48fd633e21af01af32b7ddb0532ef68dd3dd74deb2c1d2599691929617e8c09676bcbaaf7d669b88816f866f1db2
6a936580d1c42576f627d5fac5423ec7af88e547 ci: remove RUN_SECURITY_TESTS (fanquake)
Pull request description:
We no-longer run any security/symbol checks in the CI, and doubt we will in future (if we do, it'll be via Guix, where this var would be redundant in any case). The CI environment doesn't (exactly) match the release build environment (and is semi-regularly changing), and the binaries produced in the CI don't match how we build release binaries, so there is no point trying to run these checks, especially as we add more involved tests, i.e #26953.
ACKs for top commit:
josibake:
code review ACK 6a936580d1
TheCharlatan:
ACK 6a936580d1c42576f627d5fac5423ec7af88e547
Tree-SHA512: c0eec61a4b873bac487ba9321b50116a215b4796bd7d416d98ffcd09969dbf635c2cb5aeb225c89d1e6462838fa2a48565048ebe730f48d76d3db46b64855a91
98ea79841177cf7492ec1de1ba1979f75390c63b ci, iwyu: Double maximum line length for includes (Hennadii Stepanov)
Pull request description:
This PR makes the IWYU output in the CI 'tidy' task more useful by avoiding most cases where a comment ends with an ellipsis like that:
```
#include "primitives/transaction.h" // for CTxIn, CMutableTransaction, CTra...
```
ACKs for top commit:
TheCharlatan:
ACK 98ea79841177cf7492ec1de1ba1979f75390c63b
Tree-SHA512: 25195ccb2095884b23586416b86999ebc42577c6d777abdbd176a704fa75c64deb91fa61cd91d570a5408dd459e930e53bc70d963b76c73fca7a800e74b1bdbf
This requires a linux kernel of 3.17.0+, which seems entirely
reasonable. 3.17 went EOL in 2015, and the last supported 3.x kernel
(3.16) went EOL > 4 years ago, in 2020. For reference, the current
oldest maintained kernel is 4.14 (released 2017, EOL Jan 2024).
Support for `getrandom()` (and `getentropy()`) was added to
glibc 2.25, https://sourceware.org/legacy-ml/libc-alpha/2017-02/msg00079.html,
and we already require 2.27+.
All that being said, I don't think you would encounter a current day
system, running with kernel headers older than 3.17 (released 2014) but
also having a glibc of 2.27+ (released 2018).
Remove it. Make this change, so in a future commit, we can
combine #ifdefs, and avoid duplicate <sys/random.h> includes once we
switch to using getrandom directly.
Also remove the comment about macOS 10.12. We already require macOS >
10.15, so it is redundant.
4bfcbbfd4a9749f7954e37a7447b5f047465bdf8 doc: remove Security section from build-unix.md (fanquake)
Pull request description:
Our compile documentation isn't the right place for generic binary hardening notes, which are neither particularly Bitcoin-Core specific, or as relevant as they might have once been, i.e non-executable stacks are now just the norm.
Just remove the notes for now, if someone has something more interesting/Bitcoin Core specific, it could be added in separate documentation in the future (maybe into the devwiki or similar).
Split from https://github.com/bitcoin/bitcoin/pull/27685#discussion_r1196517868.
ACKs for top commit:
jarolrod:
ACK 4bfcbbfd4a9749f7954e37a7447b5f047465bdf8
Tree-SHA512: 01b523ba40353d6cafb5dbd6540b514d83a2dc4e462a068fb390ca7de45b78e4a329367f70527f1824006ebe43f8caeea4ad05ec60556d5a5bd0143e27bf6581
6b605b91c1faf2c7f7cc0c9d39b4fcfd66dc2965 [fuzz] Add MiniMiner target + diff fuzz against BlockAssembler (glozow)
3f3f2d59ea2946a7b7cc8cb0222fb602d62645d0 [unit test] GatherClusters and MiniMiner unit tests (glozow)
59afcc83548ea67a863dac7b75d000bc8f6a7023 Implement Mini version of BlockAssembler to calculate mining scores (glozow)
56484f0fdc44261e723563f59df886d5acdd851f [mempool] find connected mempool entries with GatherClusters(…) (glozow)
Pull request description:
Implement Mini version of BlockAssembler to calculate mining scores
Run the mining algorithm on a subset of the mempool, only disturbing the
mempool to copy out fee information for relevant entries. Intended to be
used by wallet to calculate amounts needed for fee-bumping unconfirmed
transactions.
From comments of sipa and glozow below:
> > In what way does the code added here differ from the real block assembly code?
>
> * Only operates on the relevant transactions rather than full mempool
> * Has the ability to remove transactions that will be replaced so they don't impact their ancestors
> * Does not hold mempool lock outside of the constructor, makes copies of the entries it needs instead (though I'm not sure if this has an effect in practice)
> * Doesn't do the sanity checks like keeping weight within max block weight and `IsFinalTx()`
> * After the block template is built, additionally calculates fees to bump remaining ancestor packages to target feerate
ACKs for top commit:
achow101:
ACK 6b605b91c1faf2c7f7cc0c9d39b4fcfd66dc2965
Xekyo:
> ACK [6b605b9](6b605b91c1) modulo `miniminer_overlap` test.
furszy:
ACK 6b605b91 modulo `miniminer_overlap` test.
theStack:
Code-review ACK 6b605b91c1faf2c7f7cc0c9d39b4fcfd66dc2965
Tree-SHA512: f86a8b4ae0506858a7b15d90f417ebceea5038b395c05c825e3796123ad3b6cb8a98ebb948521316802a4c6d60ebd7041093356b1e2c2922a06b3b96b3b8acb6
