Bitcoin Core

Merge bitcoin/bitcoin#32414: validation: periodically flush dbcache during reindex-chainstate

c1e554d3e5834a140f2a53854018499a3bfe6822 refactor: consolidate 3 separate locks into one block (Andrew Toth)
41479ed1d23ea752d0ce14c2cf5627f43bceb722 test: add test for periodic flush inside ActivateBestChain (Andrew Toth)
84820561dcb2d156d1a1151a480fc1be6649cae4 validation: periodically flush dbcache during reindex-chainstate (Andrew Toth)

Pull request description:

After #30611 we periodically do a non-erasing flush of the dbcache to disk roughly every hour during IBD.
The intention was to also do this periodic flush during reindex-chainstate, so we would not risk losing progress during a system failure when reindexing with a high dbcache value.

It was discovered that reindex-chainstate does not perform a PERIODIC flush until it has already reached the tip. Since reindexing to tip usually happens within 24 hours, this behaviour was unnoticed with the previous periodic flush interval. Note that reindex-chainstate still does IF_NEEDED flushes during `ConnectBlock`, so this also would not be noticed when running with a lower dbcache value.

This patch moves the PERIODIC flush from after the outer loop in `ActivateBestChain` to inside the outer loop after we release `cs_main`. This will periodically flush during IBD, reindex-chainstate, and steady state.

ACKs for top commit:
l0rinc:
ACK c1e554d3e5834a140f2a53854018499a3bfe6822
achow101:
ACK c1e554d3e5834a140f2a53854018499a3bfe6822
sipa:
utACK c1e554d3e5834a140f2a53854018499a3bfe6822

Tree-SHA512: c447ad03e16c9978b8ed2c285b38e1b4c56e7778ab93b6f64435116f47b8931017f5f56ab53eb61656693146aaced776f666af573a41ab28e8f2b6d8657fa756

rest: allow reading partial block data from storage

It will allow fetching specific transactions using an external index,
following https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3267485313.

Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

blockstorage: return an error code from `ReadRawBlock()`

It will enable different error handling flows for different error types.

Also, `ReadRawBlockBench` performance has decreased due to no longer reusing a vector
with an unchanging capacity - mirroring our production code behavior.

Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

blockstorage: allow reading partial block data from storage

It will allow fetching specific transactions using an external index,
following https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3267485313.

No logging takes place in case of an invalid offset/size (to avoid spamming the log),
by using a new `ReadRawError::BadPartRange` error variant.

Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

test: adjust `ComputeMerkleRoot` tests

Update the integer fuzz test to move the vector into `ComputeMerkleRoot`, matching production usage patterns and avoiding unnecessary copies.

Update `merkle_test_BlockWitness` to use an odd number of transactions to ensure the test covers the scenario where leaf duplication occurs. Also switch to `GetWitnessHash` to match `BlockWitnessMerkleRoot` semantics.
The manual vector setup retains the exact-size `resize` to explicitly verify the behavior against the calculated root.

fuzz: exercise `ComputeMerkleRoot` without mutated parameter

Co-authored-by: sedited <seb.kung@gmail.com>

util: Add Unexpected::error()

This is not needed, but a bit closer to the std lib.

test: refactor: Use BOOST_CHECK_EQUAL over BOOST_CHECK ==

Set bugprone-unused-return-value.AllowCastToVoid

It only makes sense to turn this off with C++26, which introduces the _
placeholder.

depends: Boost 1.90.0

test: Detect truncated download in get_previous_releases.py

refactor: replace manual promise with SyncWithValidationInterfaceQueue

net: Cache -capturemessages setting

refactor: extract shared `SipHash` state into `SipHashState`

Split the repeated `SipHash` v[0..3] initialization into a small `SipHashState` helper that is used by both `CSipHasher` and `PresaltedSipHasher`.

Added explanatory comments to clarify behavior, documenting the equivalence of `PresaltedSipHasher` `operator()` overloads to `CSipHasher` usage.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

optimization: cache `PresaltedSipHasher` in `CBlockHeaderAndShortTxIDs`

Replaces separate `shorttxidk0`/`shorttxidk1` members with a cached `PresaltedSipHasher`, so `GetShortID()` reuses the precomputed `SipHash` state instead of rebuilding it on every call.

`CBlockHeaderAndShortTxIDs` was never intended to be used before `FillShortTxIDSelector()` runs; doing so already relied on indeterminate salt values.
The new `Assert(m_hasher)` just makes this invariant explicit and fails fast if the object is used in an uninitialized state.

optimization: migrate `SipHashUint256` to `PresaltedSipHasher`

Replaces standalone `SipHashUint256` with an `operator()` overload in `PresaltedSipHasher`.
Updates all hasher classes (`SaltedUint256Hasher`, `SaltedTxidHasher`, `SaltedWtxidHasher`) to use `PresaltedSipHasher` internally, enabling the same constant-state caching optimization while keeping behavior unchanged.

Benchmark was also adjusted to cache the salting part.

optimization: introduce `PresaltedSipHasher` for repeated hashing

Replaces the `SipHashUint256Extra` function with the `PresaltedSipHasher` class that caches the constant-salted state (v[0-3] after XORing with keys).
This avoids redundant XOR operations when hashing multiple values with the same keys, benefiting use cases like `SaltedOutpointHasher`.

This essentially brings the precalculations in the `CSipHasher` constructor to the `uint256`-specialized SipHash implementation.

> cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release && cmake --build build -j$(nproc) && build/src/bench/bench_bitcoin -filter='SaltedOutpointHasherBench.*' -min-time=10000

> C++ compiler .......................... AppleClang 16.0.0.16000026

| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 57.27 | 17,462,299.19 | 0.1% | 11.02 | `SaltedOutpointHasherBench_create_set`
| 11.24 | 88,997,888.48 | 0.3% | 11.04 | `SaltedOutpointHasherBench_hash`
| 13.91 | 71,902,014.20 | 0.2% | 11.01 | `SaltedOutpointHasherBench_match`
| 13.29 | 75,230,390.31 | 0.1% | 11.00 | `SaltedOutpointHasherBench_mismatch`

compared to master:
create_set - 17,462,299.19/17,065,922.04 - 2.3% faster
hash - 88,997,888.48/83,576,684.83 - 6.4% faster
match - 71,902,014.20/68,985,850.12 - 4.2% faster
mismatch - 75,230,390.31/71,942,033.47 - 4.5% faster

> C++ compiler .......................... GNU 13.3.0

| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 135.38 | 7,386,349.49 | 0.0% | 1,078.19 | 486.16 | 2.218 | 119.56 | 1.1% | 11.00 | `SaltedOutpointHasherBench_create_set`
| 23.67 | 42,254,558.08 | 0.0% | 247.01 | 85.01 | 2.906 | 4.00 | 0.0% | 11.00 | `SaltedOutpointHasherBench_hash`
| 58.95 | 16,962,220.14 | 0.1% | 446.55 | 211.74 | 2.109 | 20.86 | 1.4% | 11.01 | `SaltedOutpointHasherBench_match`
| 76.98 | 12,991,047.69 | 0.1% | 548.93 | 276.50 | 1.985 | 20.25 | 2.3% | 10.72 | `SaltedOutpointHasherBench_mismatch`

compared to master:
create_set - 7,386,349.49/7,312,133.16 - 1% faster
hash - 42,254,558.08/41,978,882.62 - 0.6% faster
match - 16,962,220.14/16,549,695.42 - 2.4% faster
mismatch - 12,991,047.69/12,713,595.35 - 2% faster

Co-authored-by: sipa <pieter@wuille.net>

test: rename k1/k2 to k0/k1 in `SipHash` consistency tests

Aligns test variable naming with the `k0`/`k1` convention used consistently throughout the codebase for `SipHash` keys.
Also splits the single-param `SipHash` test from the one with extra, for clarity.

refactor: extract `SipHash` C0-C3 constants to class scope

Moves the `SipHash` initialization constants (C0-C3) from magic numbers to named static constexpr members of `CSipHasher`.

log: don't rate-limit "new peer" with -debug=net

Previously, when `debug=net` is enabled, we log "New [..] peer connected"
for new inbound peers with `LogInfo`. However, `LogInfo` will get
rate-limited since https://github.com/bitcoin/bitcoin/pull/32604.
When we specifically turn on `debug=net`, we don't want these log
messages to be rate-limited.

To fix this, use `LogDebug(BCLog::NET, ...)` for potentially high-
rate inbound connections. Otherwise use `LogInfo`. This means we
don't rate-limit the messages for inbound peers when `debug=net`
is turned on but will rate-limit if we created outbound at a high
rate as these are logged via `LogInfo`.

--

I ran into this message getting rate-limited on one of my monitoring
nodes with `-logsourcelocations=1`: With logsourcelocations, one of
these lines is about 338 chars (or 338 bytes) long. We rate-limit
after more than 1048576 bytes per hour, which results in about
3100 in- and outbound connections per hour. With evicted and
instantly reconnecting connections from an entity like LinkingLion,
this can be reached fairly quickly.

Co-Authored-By: Eugene Siegel <elzeigel@gmail.com>
Co-Authored-By: Anthony Towns <aj@erisian.com.au>

doc: add release note for 34031

netbase: Remove "tor" as a network specification

"tor" as a network specification was deprecated in 60dc8e4208 in favor
of "onion" and this commit removes it and updates the relevant test.

Co-authored-by: Mara van der Laan <126646+laanwj@users.noreply.github.com>

scripted-diff: LogPrintLevel(*,BCLog::Level::*,*) -> LogError()/LogWarning()

This is a minimal behavior change and changes log output from:

[net:error] Something bad happened
[net:warning] Something problematic happened

to either

[error] Something bad happened
[warning] Something problematic happened

or, when -loglevelalways=1 is enabled:

[all:error] Something bad happened
[all:warning] Something problematic happened

Such a behavior change is desired, because all warning and error logs
are written in the same style in the source code and they are logged in
the same format for log consumers.

-BEGIN VERIFY SCRIPT-

sed --regexp-extended --in-place \
's/LogPrintLevel\((BCLog::[^,]*), BCLog::Level::(Error|Warning), */Log\2(/g' \
$( git grep -l LogPrintLevel ':(exclude)src/test/logging_tests.cpp' )

-END VERIFY SCRIPT-

scripted-diff: LogPrintLevel(*,BCLog::Level::Debug,*) -> LogDebug()

This refactor does not change behavior.

-BEGIN VERIFY SCRIPT-

sed --regexp-extended --in-place \
's/LogPrintLevel\((BCLog::[^,]*), BCLog::Level::Debug,/LogDebug(\1,/g' \
$( git grep -l LogPrintLevel ':(exclude)src/test/logging_tests.cpp' )

-END VERIFY SCRIPT-

test: improve interface_ipc.py waitNext tests

As pointed out by Sjors in
https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598844209 and
https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598858386 the
original intention of having waitNext and waitTipChanged calls in the test was
to ensure that if new blocks were connected or fees were increased *during* the
waits, that the calls would wake up and return.

But the tests were written incorrectly, to generate blocks and transactions
before the wait calls instead of during the calls. So the tests were less
meaningful then they should be.

There was also a similar problem in the interruptWait test. The test was
intended to test the interruptWait method, but it was never actually calling
the method due to a missing await keyword. Instead it was testing that
miniwallet.send_self_transfer would interrupt the wait.

This commit fixes these issues by introducing a wait_and_do() helper function
to start parallel tasks and trigger an action after a wait call is started.

Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>