9341b5333a blockstorage: make block read hash checks explicit (Lőrinc)
2371b9f4ee test/bench: verify hash in `ComputeFilter` reads (Lőrinc)
5d235d50d6 net: assert block hash in `ProcessGetBlockData` and `ProcessMessage` (Lőrinc)
Pull request description:
A follow-up to https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2094072165, after which validating the hash of a read block from disk doesn't incur the cost of calculating its hash anymore.
### Summary
This PR adds explicit checks that the read block header's hash matches the one we were expecting.
### Context
After the previous PR, validating a block's hash during read operations became essentially free. This PR leverages that by requiring callers to provide a block's expected hash (or `std::nullopt`), preventing silent failures caused by corrupted or mismatched data. Most `ReadBlock` usages were updated with expected hashes and now fail on mismatch.
### Changes
* added hash assertions in `ProcessGetBlockData` and `ProcessMessage` to validate that the block read from disk matches the expected hash;
* updated tests and benchmark to pass the correct block hash to `ReadBlock()`, ensuring the hash validation is tested - or none if we already expect PoW failure;
* removed the default value for `expected_hash`, requiring an explicit hash for all block reads.
### Why is the hash still optional (but no longer has a default value)
* for header-error tests, where the goal is to trigger failures early in the parsing process;
* for out-of-order orphan blocks, where the child hash isn't available before the initial disk read.
ACKs for top commit:
maflcko:
review ACK 9341b5333a🕙
achow101:
ACK 9341b5333a
hodlinator:
ACK 9341b5333a
janb84:
re ACK 9341b5333a
Tree-SHA512: cf1d4fff4c15e3f8898ec284929cb83d7e747125d4ee759e77d369f1716728e843ef98030be32c8d608956a96ae2fbefa0e801200c333b9eefd6c086ec032e1f
14653b869b build: Find Boost in config mode (Hennadii Stepanov)
Pull request description:
The `FindBoost` module has been removed by policy [CMP0167](https://cmake.org/cmake/help/latest/policy/CMP0167.html).
ACKs for top commit:
purpleKarrot:
ACK 14653b869b
Tree-SHA512: 5ec88647af83158f9bc04b41a3b72d4da7d84a7c81af351b8dac61cdf7f2f3b34bedd6ff164f21c229f2fd442918aaf21ba7c2c81c346b64de9032aae27b10ce
9f8e7b0b3b node: cap -dbcache to 1GiB on 32-bit architectures (Antoine Poinsot)
2c43b6adeb init: cap -maxmempool to 500 MB on 32-bit systems (Antoine Poinsot)
Pull request description:
32-bit architecture is limited to 4GiB of RAM, so it doesn't make sense to set a too high value. A too high value could cause an OOM unbeknownst to the user a while after startup as mempool / dbcache fills.
ACKs for top commit:
achow101:
ACK 9f8e7b0b3b
instagibbs:
utACK 9f8e7b0b3b
dergoegge:
Code review ACK 9f8e7b0b3b
glozow:
utACK 9f8e7b0b3b
Tree-SHA512: cc7541b2c0040fc21a43916caec464dfb443af808f4e85deffa1187448ffff6edb0d69f9ebdb43915d145b8b4694d8465afe548f88da53ccebc9ce4b7c34b735
173394d951 depends: Build `qt` package for FreeBSD hosts (Hennadii Stepanov)
Pull request description:
This PR continues the work started in https://github.com/bitcoin/bitcoin/pull/23948.
Here is an excerpt from the log:
```
$ ./build/bin/bitcoin-qt -printtoconsole
2025-06-12T01:06:56Z Bitcoin Core version v29.99.0-15de25ba2a28 (release build)
2025-06-12T01:06:56Z Qt 6.7.3 (static), plugin=xcb
2025-06-12T01:06:56Z Static plugins:
2025-06-12T01:06:56Z QMinimalIntegrationPlugin, version 395008
2025-06-12T01:06:56Z QXcbIntegrationPlugin, version 395008
2025-06-12T01:06:56Z Style: fusion / QFusionStyle
2025-06-12T01:06:56Z System: FreeBSD 14.3-RELEASE, x86_64-little_endian-lp64
```
And here are the screenshots:
ACKs for top commit:
vasild:
ACK 173394d951
Tree-SHA512: 42a0bd11e4ef1a23efcfe6c4ab179dc667a076e65060891ce8358b3fe78de4e3ea33f975387d4236cc2ac620e2935b0a29c278065a47f038c66658106bf36755
8800b5acc1 cmake: Explicitly specify `Boost_ROOT` for Homebrew's package (Hennadii Stepanov)
Pull request description:
On macOS, this PR ensures that the Boost package is located at its real path rather than via the symlink in the default prefix.
A backport to 29.x is required for https://github.com/bitcoin/bitcoin/pull/32804, as this change prevents contamination of include directories by broad locations such as `/usr/local/include` or `/opt/homebrew/include`, which take precedence over Qt’s `-iframework` flags.
Below is the relevant change in the configuration logs on my macOS 15.5 `x64`:
- master branch @ ead4468748:
```
% cmake -B build
<snip>
-- Found Boost: /usr/local/include (found suitable version "1.88.0", minimum required is "1.73.0")
<snip>
```
- this PR:
```
% cmake -B build
<snip>
-- Found Boost: /usr/local/opt/boost/include (found suitable version "1.88.0", minimum required is "1.73.0")
<snip>
```
This PR is forward-compatible with the changes proposed in https://github.com/bitcoin/bitcoin/pull/32667.
ACKs for top commit:
fanquake:
ACK 8800b5acc1 Checked that this plus #32805fixes#31009
Tree-SHA512: 114bd945ec0c06a8d15b565e5b9aafc3bcfdf2a4ba4400e072b8e31053dff0f9ef423b941ee1dff2113f83e08f7fada728383ae88b3ec380b5c3e40553205f7d
6c2538d5bf depends: Bump boost to 1.88.0 and use new CMake buildsystem (Cory Fields)
Pull request description:
Originally #30434.
This has a few advantages over the old method of simply copying headers:
- Installs proper CMake files which can be picked up by our buildsystem
- Only installs necessary headers, not all of Boost
Pulls in upstreamed https://github.com/boostorg/test/pull/445.
ACKs for top commit:
willcl-ark:
tACK 6c2538d5bf
hebasto:
re-ACK 6c2538d5bf, only rebased since my previous [review](https://github.com/bitcoin/bitcoin/pull/32665#pullrequestreview-2891203225).
Tree-SHA512: fc3fce77b21c8ea500370841f44f1cc87e0bb09cdde55f75d2f90853cb06a6f8c73ac6ca9ca3e91a879e9f95dd59aa40254c1b04e7a168c52fa1b31cc5b7f537
ead4468748 cmake: Use `HINTS` instead of `PATHS` in `find_*` commands (Hennadii Stepanov)
Pull request description:
According to the CMake documentation, `HINTS` "should be paths computed by system introspection, such as a hint provided by the location of another item already found", which is precisely the case in the `FindQRencode` module.
Entries in `HINTS` are searched before those in `PATHS`. On macOS, Homebrew’s `libqrencode` will therefore be located at its real path rather than via the symlink in the default prefix.
A backport to 29.x is required for https://github.com/bitcoin/bitcoin/pull/32804, as this change prevents contamination of include directories by broad locations such as `/usr/local/include` or `/opt/homebrew/include`, which take precedence over Qt’s `-iframework` flags.
Below is the relevant change in the configuration logs on my macOS 15.5 `x64`:
- master branch @ ead4468748:
```
% cmake -B build -DBUILD_GUI=ON
<snip>
-- Found QRencode: /usr/local/lib/libqrencode.dylib (found version "4.1.1")
<snip>
```
- this PR:
```
% cmake -B build -DBUILD_GUI=ON
<snip>
-- Found QRencode: /usr/local/Cellar/qrencode/4.1.1/lib/libqrencode.dylib (found version "4.1.1")
<snip>
```
ACKs for top commit:
fanquake:
ACK ead4468748
Tree-SHA512: 1f0b04e3efeb7fe3efbb969be911abbcf56030d715acd87c0fbaf24422cdf1122f169e32242571256916c96a159212842e1e73092145c63ecc495ce429c6e587
dd8447f70f test: fix catchup loop in outbound eviction functional test (Sebastian Falbesoner)
Pull request description:
In the course of working on an equivalent of #32421 for the `CBlockHeader` class, I noticed that the [catchup loop in the outbound eviction functional test](19765dca19/test/functional/p2p_outbound_eviction.py (L86-L103)) currently has a small flaw: the contained waiting for a `getheaders` message
19765dca19/test/functional/p2p_outbound_eviction.py (L98-L99)
only waits for _any_ such message instead of one with the intended block hash after the first iteration. The reason is that the `prev_prev_hash` variable is set incorrectly, since the `tip_header` instance is not updated and its field `.hash` is None [1]. Fix that by updating `tip_header` after generating a new block and also use the correct field on it -- we want the tip header's previous hash (`.hashPrevBlock`), which will be the previous-previous hash in the next iteration as intended.
Can be demonstrated by adding a debug output for `prev_prev_hash`, e.g.
```diff
diff --git a/test/functional/p2p_outbound_eviction.py b/test/functional/p2p_outbound_eviction.py
index 30ac85e32f..9886a49512 100755
--- a/test/functional/p2p_outbound_eviction.py
+++ b/test/functional/p2p_outbound_eviction.py
@@ -85,6 +85,7 @@ class P2POutEvict(BitcoinTestFramework):
self.log.info("Keep catching up with the old tip and check that we are not evicted")
for i in range(10):
+ print(f"i={i}, prev_prev_hash={prev_prev_hash}")
# Generate an additional block so the peers is 2 blocks behind
prev_header = from_hex(CBlockHeader(), node.getblockheader(best_block_hash, False))
best_block_hash = self.generateblock(node, output="raw(42)", transactions=[])["hash"]
```
master branch
```
...
i=0, prev_prev_hash=21722572577213525620063947414919931742473663114977483853465070858884938201585
i=1, prev_prev_hash=None
i=2, prev_prev_hash=None
i=3, prev_prev_hash=None
i=4, prev_prev_hash=None
i=5, prev_prev_hash=None
i=6, prev_prev_hash=None
i=7, prev_prev_hash=None
i=8, prev_prev_hash=None
i=9, prev_prev_hash=None
...
```
PR branch
```
...
i=0, prev_prev_hash=21722572577213525620063947414919931742473663114977483853465070858884938201585
i=1, prev_prev_hash=23204083306104595181276643925327085197417756603258684897360269464191973063397
i=2, prev_prev_hash=18117007775254206852722585270408843074799046031613422902091537272077477361634
i=3, prev_prev_hash=30556804635951812756130312631227721973553160707632138130845362630877961299882
i=4, prev_prev_hash=16476515948153779819467376247405243058769281687868039119037064816106574626111
i=5, prev_prev_hash=14965506521435221774966695805624206855826023174786191695076697927307467053159
i=6, prev_prev_hash=14510815979277079515923749862202324542606166669768865640616202929053689167149
i=7, prev_prev_hash=15360268707191667685151951417759114642582372006627142890517655217275478262166
i=8, prev_prev_hash=55984929479619644661389829786223559362979512070332438490054115824374865094074
i=9, prev_prev_hash=6591573629906616262191232272909118561529534571119028248829355592878183757083
...
```
[1] that's in my opinion another example how caching hashes is confusing and easy to be misused; it's better to remove it and just compute the hash on-the-fly, so returning None is not even possible anymore
ACKs for top commit:
maflcko:
lgtm ACK dd8447f70f
mzumsande:
Code Review ACK dd8447f70f
pablomartin4btc:
cr-ACK dd8447f70f
Tree-SHA512: bd8e786b52e3e96661453006140d6b8fad5a35f1c8d38243c61df52b19c97cd3800404745a2f9603bcdf0006e9780b4f15f8f7e4fa34ff07d52dba04d87b68d0
c3fe85e2d6 wallet, rpc, test: Remove deprecated getunconfirmedbalance (Ava Chow)
0ec255139b wallet, rpc: Remove deprecated balances from getwalletinfo (Ava Chow)
Pull request description:
`getwalletinfo` result fields `balance`, `immature_balance`, and `unconfirmed_balance`, and the `getunconfirmedbalance` RPC have all been deprecated since 0.19.0. It's been long enough that they should either be removed or undeprecated. The functionality provided by these RPCs is provided by `getbalances`.
ACKs for top commit:
davidgumberg:
ACK c3fe85e2d6
rkrux:
ACK c3fe85e2d6
BrandonOdiwuor:
ACK c3fe85e2d6 removing the deprecated `balance, unconfirmed_balance, immature_balance` fields from `getwalletinfo` and `getunconfirmedbalance` RPCs, as this infomation can be found on the `getbalances` RPC
w0xlt:
reACK c3fe85e2d6
Tree-SHA512: c7c4acfd9cabc7517ba813b95281a6c6a717a417312afd9346298669b4f7bd37724ad977148ce42db7fd47fc3d1f5a8482d8ff2e7b9cb74756b171a5b8b91ef2
47237cd193 wallet, rpc: Output wallet flags in getwalletinfo (Ava Chow)
bc2a26b296 wallet: Add GetWalletFlags (Ava Chow)
69f588a99a wallet: Set upgraded descriptor cache flag for newly created wallets (Ava Chow)
Pull request description:
Newly created wallets will always have an upgraded descriptor cache, so set those.
Also, to verify this behavior, add a new `flags` field to `getwalletinfo` and check that in the functional tests.
Split from #32489
ACKs for top commit:
Sjors:
ACK 47237cd193
w0xlt:
ACK 47237cd193
rkrux:
ACK 47237cd193
Tree-SHA512: 97c7f85b858efe5ced9b8aafb6cd7c1a547de6f8013b82bfc75bc567cf73c9db5e168e3980355756541305520022fd776b8d4d240d3fb34ed86c27d2acaf4863
9eb2c82e7c walletdb: Remove unused upgraded_txs (Ava Chow)
c668033709 wallet: Remove unused fTimeReceivedIsTxTime (Ava Chow)
Pull request description:
`CWalletTx::fTimeReceivedIsTxTime` is no longer used and can be removed. This additionally allows the removal of the `upgraded_txs` loop in `LoadWallet`.
ACKs for top commit:
maflcko:
lgtm ACK 9eb2c82e7c
Eunovo:
ACK 9eb2c82e7c
davidgumberg:
ACK 9eb2c82e7c
PeterWrighten:
ACK 9eb2c82e7c
rkrux:
ACK 9eb2c82e7c
w0xlt:
ACK 9eb2c82e7c
Tree-SHA512: 05cf3a50f0d8ab6ef423ad1113c5ce6f45bfdc90e2c0dcf61c2dceced2465502e574b4b5b0091fcbb4bdd2182f8d69224f1e5516c7c505de07102b84a5f40e9c
This has a few advantages over the old method of simply copying headers:
- Installs proper cmake files which can be picked up by our buildsystem
- Only installs necessary headers, not all of boost
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
e27a94596f build: add root dir to CMAKE_PREFIX_PATH (will)
Pull request description:
Fixes: #32428
Nix patches `cmake` to remove the root directory `/` from `CMAKE_PREFIX_PATH`:
428b49b28e/pkgs/by-name/cm/cmake/001-search-path.diff (L10)
Without this, and when using the toolchain for depends builds, cmake's `find_path()` and `find_package()` do not know where to find dependencies, causing issues like as seen in #32428
Adding this path back is harmless on other systems, and fixes the toolchain for Nix users.
As described in https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2991258328 I think this can be taken as a temporary fix whilst a longer-term solution is worked on.
ACKs for top commit:
josibake:
reACK e27a94596f
hebasto:
ACK e27a94596f, I have reviewed the code and it looks OK.
janb84:
reACK e27a94596f
Tree-SHA512: f299f2bab2620179518da866cbb7992d41d142ad42e79c14496e72f725a1dc60698b3e4b1daf45d28f71f32a23f0c8d7b4f6c6cf33aeedf322b7ef565b70b4af
According to the CMake documentation, `HINTS` "should be paths computed
by system introspection, such as a hint provided by the location of
another item already found", which is precisely the case in the
`FindQRencode` module.
Entries in `HINTS` are searched before those in `PATHS`. On macOS,
Homebrew’s `libqrencode` will therefore be located at its real path
rather than via the symlink in the default prefix.
fa68dcb207 ci: Add missing errexit to lint CI install (MarcoFalke)
fa535a6de7 ci: Allow running CI in worktrees (MarcoFalke)
faf6a04597 ci: Clean UID/GID mismatch (MarcoFalke)
Pull request description:
Fixes#30028 (modulo lint and tidy CI).
The error on current master in a worktree is:
```
$ git worktree add ./main origin/master && cd ./main
$ MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_mac_cross.sh" ./ci/test_run_all.sh
...
+ git config --global ci.base-install-done true
fatal: not a git repository: /root/b-c-ci/.git/worktrees/main
```
So just use a plain file, instead of git. Also, enable pipefail while touching this bash script.
ACKs for top commit:
willcl-ark:
tACK fa68dcb207
Tree-SHA512: 0ce360a80883b4aa655fe8a99c38eb54a465b17c7cdb0a69a2d886ff78da32d6af996412ffc5b0db0322acafa9650619838573484de8243dc41594a04a6e17ec
Nix patches cmake to remove the root directory `/` from
`CMAKE_SYSTEM_PREFIX_PATH`:
428b49b28e/pkgs/by-name/cm/cmake/001-search-path.diff (L10)
Without this, and when using the toolchain for depends builds, cmake's
`find_path()` and `find_package()` do not know where to find
dependencies, causing issues like:
https://github.com/bitcoin/bitcoin/issues/32428
Adding this path back via CMAKE_PREFIX_PATH is harmless on other
systems, and fixes the toolchain for Nix users.
We append the `/` dir a maximum of once, as the toolchain may be called
repeatedly during builds.
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: josibake <josibake@protonmail.com>
3473986fe1 contrib: tracing: Correctly read msg type in p2p_monitor.py (David Gumberg)
Pull request description:
This fixes a bug in the contrib tracing script `p2p_monitor.py`. currently the script fails to read the `msg_type` of inbound and outbound messages, which is useful in the per-peer message view.
<details>
<summary>Screenshot of p2p_monitor.py on master</summary>
</details>
<details>
<summary>Screenshot of p2p_monitor.py on this branch</summary>
</details>
ACKs for top commit:
yuvicc:
ACK 3473986fe1
janb84:
ut ACK 3473986fe1
0xB10C:
ACK 3473986fe1
Tree-SHA512: 94da0dc35072933a20ef693024855b3c382fc6f5ae0a3108d092d7aa5a4004df478f5de07b80f675be13e00f3f4596b0f34c49ec1d8d2c38a15797dcf86c2a56
578ea3eedb test: round difficulty and networkhashps (Sjors Provoost)
Pull request description:
Both are rational numbers. Client software should only use them to display information to humans. Followup calculations should use the underlying values such as target.
Therefore it's not necessary to test the handling of these floating point values. Round them down to avoid spurious test failures.
Fixes#32515
ACKs for top commit:
Prabhat1308:
Code Review ACK [`578ea3e`](578ea3eedb)
achow101:
ACK 578ea3eedb
w0xlt:
Code review ACK 578ea3eedb
janb84:
ACK 578ea3eedb
Tree-SHA512: 5fc63c73ad236b7cd55c15da0f1d1e6b45e4289d252147a86717bf77d79f897f42c3e38aa514df6a4a8deca10c87a8710b61b454c533ad56b0daf738365f426c
b184f5c87c test: update BIP340 test vectors and implementation (variable-length messages) (Sebastian Falbesoner)
Pull request description:
This PR updates the Schnorr signatures implementation in the functional test framework to the latest BIP changes (see https://github.com/bitcoin/bips/pull/1446,commit 200f9b26fe0a2f235a2af8b30c4be9f12f6bc9cb) and syncs the [test vectors](https://github.com/bitcoin/bips/blob/master/bip-0340/test-vectors.csv) accordingly. Practically, we probably don't need non-32-bytes message signing/verifying any time soon, but it seems good practice anyways to update.
ACKs for top commit:
stratospher:
ACK b184f5c.
achow101:
ACK b184f5c87c
real-or-random:
utACK b184f5c87c
jonasnick:
utACK b184f5c87c
Tree-SHA512: b566823aa0f1cd7151215178c57551d772b338d022ccb2807a0df2670df6d59c4b63a6fc936708ccf2922c7e59f474f544adaafc4aea731bfd896250c0d45fa6
272cd09b79 log: Use warning level while scanning wallet dir (MarcoFalke)
1777644367 qa, wallet: Verify warning when failing to scan (Hodlinator)
893e51ffeb wallet: Correct dir iteration error handling (Hodlinator)
Pull request description:
Make wallet DB properly detect and report failure to scan wallet directory. Seems to have been broken since moving from Boost to `std::filesystem`.
Found while reviewing: https://github.com/bitcoin/bitcoin/pull/31410#pullrequestreview-2604068753
ACKs for top commit:
achow101:
ACK 272cd09b79
maflcko:
re-ACK 272cd09b79 🍽
rkrux:
tACK 272cd09b79
Tree-SHA512: 969afde2e37f885ed0c823dc36d2dbeaa0378639849c6a26f8ac67b4f1997eea95bbcae6d58aef5b716807210f37eb166c0cda7ba1d6caffd34249970833af3a
cd1ae1b4df fuzz: wallet: remove FundTx from FuzzedWallet (brunoerg)
Pull request description:
`FundTx` was used by the `wallet_notifications` target which we recently removed. So it's now unused and can be removed.
ACKs for top commit:
maflcko:
lgtm ACK cd1ae1b4df
kevkevinpal:
ACK [cd1ae1b](cd1ae1b4df)
dergoegge:
utACK cd1ae1b4df
Tree-SHA512: 909cc4c8a0ac2a5f8844993ccf0e725021932888da3591925799145daf9196eadfcd0ebbc74a44f4a245074ded4cb8c3c099513f109ce2681dceff36b5f74bcc
e285e691b7 test: Fix list index out of range error in feature_bip68_sequence.py (zaidmstrr)
Pull request description:
Fixes [#32334](https://github.com/bitcoin/bitcoin/issues/32334)
The test `feature_bip68_sequence.py` fails with `IndexError: list index out of range` error due to a mismatch between the number of inputs requested (at random) and the number of UTXOs available. The error is reproducible with the randomseed:
```
$ ./build/test/functional/feature_bip68_sequence.py --randomseed 6169832640268785903
```
This PR adds a valid upper bound to randomly select the inputs.
ACKs for top commit:
maflcko:
lgtm ACK e285e691b7
Prabhat1308:
re-ACK [`e285e69`](e285e691b7)
theStack:
ACK e285e691b7
Tree-SHA512: 2e5e19d5db2880915f556ed4444abed94e9ceb1ecee5f857df5616040c850dae682aaa4ade3060c48acb16676df92ba81c3af078c1958965e9e874e7bb489388
fa94fd53c9 doc: Explain how to fetch commits directly (MarcoFalke)
Pull request description:
This is often needed, and works better than the existing refspec documentation, because even commits that have been force-pushed away can be fetched (as long as they are not garbage collected on the remote).
ACKs for top commit:
Sjors:
ACK fa94fd53c9
l0rinc:
ACK fa94fd53c9
willcl-ark:
ACK fa94fd53c9
rkrux:
ACK fa94fd53c9
janb84:
ACK fa94fd53c9
Tree-SHA512: b68c0c612e13f501ad4c1c709502e060b0a2d0eb55ef888c7466e2a10bdf3ca63d81b8bd7927de49cde9e29f0b06f8233d51b99d015ae0b39d556854be542b8a
53a996f122 doc: fix transifex 404s (fanquake)
Pull request description:
https://www.transifex.com/bitcoin/bitcoin/ is now a 404.
ACKs for top commit:
maflcko:
lgtm ACK 53a996f122
hebasto:
ACK 53a996f122, I've verified all the links.
Tree-SHA512: 8e698c83095a3d3a225b0bf2ee9c39ad434b2917ead4271ff39a282cea6283710091d1e8b91edafd280bf356dec2bdbe42981aafe4d64f623a975232c5ca848c
8ee8a951c2 doc: taproot became always active in v24.0 (Sjors Provoost)
Pull request description:
Split from #26201.
ACKs for top commit:
maflcko:
lgtm ACK 8ee8a951c2
janb84:
ACK 8ee8a951c2
Tree-SHA512: 1ac6994c6775ca5423f022d1e02e3d531fb7fa295be9940355b8aa9d173787a8d65945a0cf976ab344bcaa3ea8a0f3aa6f8da851325bf475e59375981b115cab
a18e572328 test: more template verification tests (Sjors Provoost)
10c908808f test: move gbt proposal mode tests to new file (Sjors Provoost)
94959b8dee Add checkBlock to Mining interface (Sjors Provoost)
6077157531 ipc: drop BlockValidationState special handling (Sjors Provoost)
74690f4ed8 validation: refactor TestBlockValidity (Sjors Provoost)
Pull request description:
This PR adds the IPC equivalent of the `getblocktemplate` RPC in `proposal` mode.
In order to do so it has `TestBlockValidity` return error reasons as a string instead of `BlockValidationState`. This avoids complexity in IPC code for handling the latter struct.
The new Mining interface method is used in `miner_tests`.
It's not used by the `getblocktemplate` and `generateblock` RPC calls, see https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2096473337
The `inconclusive-not-best-prevblk` check is moved from RPC
code to `TestBlockValidity`.
Test coverage is increased by `mining_template_verification.py`.
Superseedes #31564
## Background
### Verifying block templates (no PoW)
Stratum v2 allows miners to generate their own block template. Pools may wish (or need) to verify these templates. This typically involves comparing mempools, asking miners to providing missing transactions and then reconstructing the proposed block.[^0] This is not sufficient to ensure a proposed block is actually valid. In some schemes miners could take advantage of incomplete validation[^1].
The Stratum Reference Implementation (SRI), currently the only Stratum v2 implementation, collects all missing mempool transactions, but does not yet fully verify the block.[^2]. It could use the `getblocktemplate` RPC in `proposal` mode, but using IPC is more performant, as it avoids serialising up to 4 MB of transaction data as JSON.
(although SRI could use this PR, the Template Provider role doesn't need it, so this is _not_ part of #31098)
[^0]: https://github.com/stratum-mining/sv2-spec/blob/main/06-Job-Declaration-Protocol.md
[^1]: https://delvingbitcoin.org/t/pplns-with-job-declaration/1099/45?u=sjors
[^2]: https://github.com/stratum-mining/stratum/blob/v1.1.0/roles/jd-server/src/lib/job_declarator/message_handler.rs#L196
ACKs for top commit:
davidgumberg:
reACK a18e572328
achow101:
ACK a18e572328
TheCharlatan:
ACK a18e572328
ryanofsky:
Code review ACK a18e572328 just adding another NONFATAL_UNREACHABLE since last review
Tree-SHA512: 1a6c29f45a1666114f10f55aed155980b90104db27761c78aada4727ce3129e6ae7a522d90a56314bd767bd7944dfa46e85fb9f714370fc83e6a585be7b044f1
rsync --archive will preserve owner and group, which is then required to
be handled by adding a git safe.directory workaround.
Remove the need for the workaround by only preserving permissions during
the recursive rsync copy.
