c11508406e doc: Update docs that refer to -maxconnections (Martin Zumsande)
69ce0dba2a test: add test that EvictTxPeerIfFull only evicts tx-relaying peers (brunoerg)
3ed7f06418 p2p: trigger possible eviction if we support bloom filters and change a peer to tx relay (Martin Zumsande)
0bd3d3dfa5 init: make inbound tx relay percentage configurable (Amiti Uttarwar)
cc59aee196 test: add functional test for inbound maxconnection limits (Amiti Uttarwar)
1b76e04736 net: increase inbound capacity for block-relay-only connections (Martin Zumsande)
87bca1c2ad net: add options to AttemptToEvictConnection (Martin Zumsande)
Pull request description:
This is joint work with amitiuttarwar.
See issue #28462 for a broader discussion on increasing the number of block-relay-only connections independent of this particular implementation proposal.
We suggest to increase the number of inbound slots allocated to block-relay-only peers by increasing the default maximum connections from 125 to 200, with 50% of inbound slots accessible for tx-relaying peers.
This is a prerequisite for being able to increase the default number of outgoing block-relay-only peers later, because the current inbound capacity of the network is not sufficient.
In order to account for incoming tx-relaying peers separately from incoming block-relay peers, changes to the inbound eviction logic are necessary.
See the next post in this thread for a more detailed explanation and motivation of the changes.
ACKs for top commit:
instagibbs:
ACK c11508406e
achow101:
ACK c11508406e
dergoegge:
crACK c11508406e
marcofleon:
ACK c11508406e
Tree-SHA512: c71e1481eb235429a6c9d7ce771c7bf825f850b135e904ccfa3505112628fef4188b560d0be0847c968e5ece43c1518590069b7e6e2480790d3ef1ce07d1ac38
05c35c402c refactor: Make all `const static` class members `constexpr` (rustaceanrob)
Pull request description:
Found in #35713. If a `static class` member is not inlined or `constexpr`, the linker will fail when attempting to ODR-use the constant (passing as `const T&`). These can be fixed by finding all member variables that are `const` qualified and inlining them with `constexpr`. There is a clang-tidy pull request that would lint these callsites: https://github.com/llvm/llvm-project/pull/162741
A script was used to modify these sites, however it cannot run as a scripted-diff because it uses clang-query and a build folder.
The script only queries for integer and enumeration types, as other data members would have to be marked `constexpr` or `inline` from what I understand: https://en.cppreference.com/cpp/language/static#Constant_static_members
Removing the ZMQ forward declaration was a clang-tidy lint.
<details>
<summary>The script used to find these sites, LLM assisted:</summary>
```
set -uxo pipefail
cd "$(git rev-parse --show-toplevel)"
BUILD=${BUILD:-build}
if [ ! -f "${BUILD}/compile_commands.json" ]; then
echo "error: ${BUILD}/compile_commands.json not found. Run cmake -B ${BUILD} first." >&2
exit 1
fi
if ! command -v clang-query >/dev/null; then
echo "error: clang-query not on PATH. Install clang-tools." >&2
exit 1
fi
if ! git diff --quiet || ! git diff --cached --quiet; then
echo "error: working tree has uncommitted changes. Commit or stash first." >&2
exit 1
fi
MATCHER='match varDecl(hasParent(cxxRecordDecl()),
hasType(qualType(isConstQualified(),
anyOf(hasCanonicalType(isInteger()),
hasDeclaration(enumDecl())))),
hasInitializer(expr()),
unless(isConstexpr()),
isExpansionInFileMatching("/src/"))'
RAW=$(mktemp)
trap 'rm -f "$RAW"' EXIT
echo "Sweeping TUs (batched, may take a few minutes)..." >&2
find src -type d \( -name secp256k1 -o -name leveldb -o -name crc32c \
-o -name minisketch -o -name libmultiprocess -o -name ctaes \) -prune -o \
-name '*.cpp' -print0 \
| xargs -0 -n 50 clang-query -p "${BUILD}" \
-c 'set output diag' \
-c "${MATCHER}" \
>>"$RAW" || true
ROOT=$(pwd)
LOCS=$(grep -oE "${ROOT}/src/[^:]+:[0-9]+:[0-9]+:" "$RAW" \
| sed -E "s|^${ROOT}/||; s|:[0-9]+:$||" \
| sort -u)
if [ -z "$LOCS" ]; then
echo "no matches" >&2
exit 0
fi
FILTERED=""
while IFS=: read -r file line; do
case "$file" in
src/secp256k1/*|src/leveldb/*|src/crc32c/*|src/minisketch/*|src/ipc/libmultiprocess/*|src/crypto/ctaes/*) continue ;;
src/tinyformat.h) continue ;;
esac
src=$(sed -n "${line}p" "$file")
case "$src" in *inline*) continue ;; esac
FILTERED+="${file}:${line}"$'\n'
done <<<"$LOCS"
FILTERED=$(printf '%s' "$FILTERED" | sed '/^$/d')
if [ -z "$FILTERED" ]; then
echo "no matches after filtering" >&2
exit 0
fi
echo "Sites to rewrite ($(echo "$FILTERED" | wc -l)):" >&2
echo "$FILTERED" >&2
declare -A LINES
while IFS=: read -r file line; do
LINES[$file]+="${line} "
done <<<"$FILTERED"
for file in "${!LINES[@]}"; do
args=()
for line in ${LINES[$file]}; do
args+=(-e "${line}s/static const /static constexpr /")
done
sed -i "${args[@]}" "$file"
done
echo >&2
echo "===== proposed diff =====" >&2
git --no-pager diff
```
</details>
ACKs for top commit:
fanquake:
ACK 05c35c402c
sedited:
ACK 05c35c402c
Tree-SHA512: 2b823b94ddfae1a889b50ebdb6a8828d95baaa2578756daa826b6579045f7e92ea91562be96865c1df267f4dd288f91fd84ed60090e9ad38adc4efeb865cd90a
If a `static class` member is not inlined or `constexpr`, the linker
will fail when attempting to ODR-use the constant (passing as `const
T&`). These can be fixed by finding all member variables that are
`const` qualified and inlining them with `constexpr`. There is a
clang-tidy pull request that would lint these callsites: https://github.com/llvm/llvm-project/pull/162741
A script was used to modify these sites, however it cannot run as a
scripted-diff because it uses clang-query and a build folder.
The script only queries for integer and enumeration types, as other data
members would have to be marked `constexpr` or `inline` from what I
understand: https://en.cppreference.com/cpp/language/static#Constant_static_members
Removing the ZMQ forward declaration was a clang-tidy lint.
The script used to find these sites, LLM assisted:
```
set -uxo pipefail
cd "$(git rev-parse --show-toplevel)"
BUILD=${BUILD:-build}
if [ ! -f "${BUILD}/compile_commands.json" ]; then
echo "error: ${BUILD}/compile_commands.json not found. Run cmake -B ${BUILD} first." >&2
exit 1
fi
if ! command -v clang-query >/dev/null; then
echo "error: clang-query not on PATH. Install clang-tools." >&2
exit 1
fi
if ! git diff --quiet || ! git diff --cached --quiet; then
echo "error: working tree has uncommitted changes. Commit or stash first." >&2
exit 1
fi
MATCHER='match varDecl(hasParent(cxxRecordDecl()),
hasType(qualType(isConstQualified(),
anyOf(hasCanonicalType(isInteger()),
hasDeclaration(enumDecl())))),
hasInitializer(expr()),
unless(isConstexpr()),
isExpansionInFileMatching("/src/"))'
RAW=$(mktemp)
trap 'rm -f "$RAW"' EXIT
echo "Sweeping TUs (batched, may take a few minutes)..." >&2
find src -type d \( -name secp256k1 -o -name leveldb -o -name crc32c \
-o -name minisketch -o -name libmultiprocess -o -name ctaes \) -prune -o \
-name '*.cpp' -print0 \
| xargs -0 -n 50 clang-query -p "${BUILD}" \
-c 'set output diag' \
-c "${MATCHER}" \
>>"$RAW" || true
ROOT=$(pwd)
LOCS=$(grep -oE "${ROOT}/src/[^:]+:[0-9]+:[0-9]+:" "$RAW" \
| sed -E "s|^${ROOT}/||; s|:[0-9]+:$||" \
| sort -u)
if [ -z "$LOCS" ]; then
echo "no matches" >&2
exit 0
fi
FILTERED=""
while IFS=: read -r file line; do
case "$file" in
src/secp256k1/*|src/leveldb/*|src/crc32c/*|src/minisketch/*|src/ipc/libmultiprocess/*|src/crypto/ctaes/*) continue ;;
src/tinyformat.h) continue ;;
esac
src=$(sed -n "${line}p" "$file")
case "$src" in *inline*) continue ;; esac
FILTERED+="${file}:${line}"$'\n'
done <<<"$LOCS"
FILTERED=$(printf '%s' "$FILTERED" | sed '/^$/d')
if [ -z "$FILTERED" ]; then
echo "no matches after filtering" >&2
exit 0
fi
echo "Sites to rewrite ($(echo "$FILTERED" | wc -l)):" >&2
echo "$FILTERED" >&2
declare -A LINES
while IFS=: read -r file line; do
LINES[$file]+="${line} "
done <<<"$FILTERED"
for file in "${!LINES[@]}"; do
args=()
for line in ${LINES[$file]}; do
args+=(-e "${line}s/static const /static constexpr /")
done
sed -i "${args[@]}" "$file"
done
echo >&2
echo "===== proposed diff =====" >&2
git --no-pager diff
```
Permit users to change the amount of inbounds that are permitted to relay
transactions. This is particularly relevant to ensure that superusers that are
not concerned with resource usage are not artificially restricted from offering
many transaction relay slots to the network.
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
..and adjust the eviction logic.
The new default max connection number is 200, the default maximum of tx-relaying
inbounds is limited to 50% of all inbound connections.
With 11 outbound connections, that is (200 - 11) * 0.5 = 94.5.
As a result, the tx-related maximum traffic should not change
drastically.
When we receive an inbound connection and don't have space for another
full-relay peer, we now attempt to evict specifically a full-relay inbound
after receiving the version message of the new peer.
Once this commit is widely deployed, the added inbound capacity will
allow us to increase the number of outgoing block-relay-only connections.
Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
7295b8be70 chainparams: remove my testnet3 seed (Sjors Provoost)
Pull request description:
With testnet3 long deprecated, albeit still not dropped #31975, and [testnet5](https://groups.google.com/g/bitcoindev/c/kGUMTxOvdJA) on the horizon, this seems like a good time. I plan to keep it up and running until v31 is end-of-life, so no need to backport.
ACKs for top commit:
fanquake:
ACK 7295b8be70
nebula-21:
ACK 7295b8be70
willcl-ark:
ACK 7295b8be70
Tree-SHA512: 30c9ad9480a09cf48138157b5529e2878469bc629edb60c5dd2605e078647b9b9b025937936d1b6e3a980c6be3fc36c512bdd91a6864c72c3e093cd60d691860
2cf9d79d84 key: validate BIP32 seed length in CExtKey::SetSeed (Muhammad)
Pull request description:
BIP32 specifies that the seed must be between 128 and 512 bits (16 to 64 bytes). CExtKey::SetSeed currently accepts any length, which could result in weak master keys being generated.
Add an Assert at the start of SetSeed to enforce the valid seed length range as a programming invariant. The existing BIP32 test vectors already provide sufficient coverage of valid seed lengths.
To test:
> cmake --build build -j --target test_bitcoin
./build/bin/test_bitcoin --run_test=bip32_tests
Fixes#35308
ACKs for top commit:
achow101:
ACK 2cf9d79d84
sedited:
ACK 2cf9d79d84
Tree-SHA512: 0ac44f8a464172b465834e1b3edf457fc8a09eeaa89cdfb1286af577d3c90d75627af8b5c5edd66232d45c60812ebc7f6f402db6f804a56f727353b2bdca7c19
c4068cf37b test: add negative zero CSV failure script test vector (azuchi)
37edf0e233 test: add CHECKLOCKTIMEVERIFY failure-path script test vectors (azuchi)
a86a96d17b test: add CHECKSIGVERIFY/CHECKMULTISIGVERIFY failure script test vectors (azuchi)
Pull request description:
While reviewing spec coverage of `src/test/data/script_tests.json` against the script interpreter, I found two gaps that are testable within this file's harness but were never covered:
**1. `OP_CHECKSIGVERIFY` / `OP_CHECKMULTISIGVERIFY` failure paths**
`OP_CHECKSIGVERIFY` never appears anywhere in the file, and no vector expects the `CHECKSIGVERIFY` or `CHECKMULTISIGVERIFY` script errors, so the VERIFY tail of both opcodes (interpreter.cpp, `case OP_CHECKSIGVERIFY`) is untested here. This commit adds static vectors that fail the signature check with an empty signature and a valid pubkey, so each opcode returns its opcode-specific error code. The success paths require real signatures and remain covered by the auto-generated tests and functional tests.
**2. `CHECKLOCKTIMEVERIFY` (BIP65) failure paths**
`SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY` is never set by any vector: `CHECKLOCKTIMEVERIFY` only appears as an unflagged NOP, so none of the BIP65 semantics are exercised, while the equivalent CHECKSEQUENCEVERIFY section has existed since #7994. This commit adds a section mirroring the CSV tests, covering every failure path reachable in this harness:
- empty stack → `INVALID_STACK_OPERATION`
- negative operand → `NEGATIVE_LOCKTIME`
- negative zero (`0x80`), evaluated as 0 by `CScriptNum` → `UNSATISFIED_LOCKTIME` rather than `NEGATIVE_LOCKTIME`
- non-minimal encoding under MINIMALDATA → `SCRIPTNUM`
- final input nSequence (lock time requirement itself satisfied) → `UNSATISFIED_LOCKTIME`
- operand greater than the tx nLockTime → `UNSATISFIED_LOCKTIME`
- height/time type mismatch → `UNSATISFIED_LOCKTIME`
- 5-byte operand (2^32) accepted by the parser, then failing the type check → `UNSATISFIED_LOCKTIME`
Unlike CSV (where an operand with bit 31 set makes the opcode pass without calling `CheckSequence`), the CLTV success path cannot be expressed in this file, because the test harness spends with nLockTime=0 and a final nSequence; it is covered by `tx_valid.json` and functional tests instead. A comment in the JSON notes this.
**3. Negative zero vector for the existing `CHECKSEQUENCEVERIFY` section**
Following review feedback, the third commit adds the same negative-zero vector to the existing CSV section: the footgun is identical there (a re-implementation treating any operand with the sign bit set as negative would return `NEGATIVE_LOCKTIME` instead of reaching `CheckSequence`), and it keeps the two sections mirrored.
ACKs for top commit:
achow101:
ACK c4068cf37b
sedited:
ACK c4068cf37b
Tree-SHA512: e7baa9d96b0faec1115c7afb97aa2a8ac17a93d44637cd3e58ab91240b198b6d21c84a29129b586ff760f8f4ca8b1988c652b8f5ebd6ebaa0ffae6615c9aec5d
ca9ffb8e12 rpc: add OpenRPC discovery alias (willcl-ark)
ef0676f400 rpc: factor getaddressinfo embedded field docs (will)
1fb6b60560 test: add functional test for getopenrpcinfo (will)
672dd42d14 rpc: add getopenrpcinfo command (will)
f5116c587f rpc: add placeholder annotation for deprecated params (will)
26c221a980 rpc: expose RPC metadata for introspection (will)
6a1a66c180 rpc: render Type::ANY in help text instead of aborting (will)
06de34a033 rpc: erase empty map entry in removeCommand (will)
d4d64ae739 rpc: add missing string_view include to server.h (will)
Pull request description:
Fixes#29912
This PR adds a machine-readable[ OpenRPC](https://www.open-rpc.org/) 1.4.1 specification of out JSON-RPC interface, auto-generated from existing `RPCHelpMan` metadata.
There is currently no formal, machine-readable specification of the RPC API. As discussed in #29912, this has knock-on consequences:
- Client libraries re-implement the API manually, leading to bugs like unit mistakes (sats vs BTC, vB vs kvB) and missing/incorrect argument types. No existing client library fully and correctly implements the API in a type-safe manner.
- When the API changes, every downstream client must manually discover and adapt, creating downstream maintenance burden. There is no artifact they can diff between releases.
- Implementing a new client in a new language requires reading C++ source or help text and transcribing it, which is error-prone and tedious, and represents an on-going porting cost.
- Existing documentation is either stale or not machine-readable. The developer.bitcoin.org docs are wrong/outdated in places, and the bitcoincore.org/en/doc/ pages are rendered from help output but not in a standard schema format.
- (new/extra) AI/LLM tooling increasingly builds on structured API specifications. A standard spec format enables AI-assisted client generation and integration without the ambiguity of parsing human-readable help text.
This draft builds on prior art by casey and the observations by laanwj, stickies-v, kilianmh, hodlinator, and cdecker in #29912. Casey's work demonstrated that RPCHelpMan already contains all the structured information needed, which makes this feasible without duplicating any API definitions.
This differs from Casey's branches in that it uses the OpenRPC standard rather than an ad-hoc format or raw JSON Schema.
### Why OpenRPC
I seletced OpenRPC for a number of reasons:
- It's purpose-built for JSON-RPC APIs (suggested by stickies-v,nflatrea, and kilianmh).
- It wraps JSON schema for params/results, so consumers get both the method-level structure and the type-level schemas.
- Unlike OpenAPI, it is not path-centric, which better fits our single-endpoint JSON-RPC model (concern raised by hodlinator).
- Although it therefore does not cover our REST interface.
- It's _kind of_ a standard format with (_some_) existing tooling for type generation (TypeScript, Rust, Python, Go) and client scaffolding, though maturity varies by language. More importantly though, IMO, a ~standardised format is inherently more useful than any ad-hoc one: any JSON Schema validator works, any LLM can consume it directly, and anyone can write a bespoke generator against a known schema rather than parsing help text.
### Approach
`RPCHelpMan` metadata → `getopenrpcinfo` / `rpc.discover` → OpenRPC JSON
### Tradeoffs
vs an ad-hoc format OpenRPC gives us interoperability with the (admittedly surprisingly limited) tooling, documentation generators, code generators, and validators, at the cost of needing x-bitcoin-* extensions for Bitcoin-specific concepts. As Casey noted after trying both approaches, JSON Schema "is probably not a great fit". OpenRPC's method-level framing on top of JSON Schema addresses the ergonomic issues while keeping the schema benefits. After testing both, I think I agree.
Types: JSON Schema cannot natively express all Bitcoin-specific semantics. Amount result fields are represented as JSON numbers with `x-bitcoin-unit: amount`; other Bitcoin-specific distinctions remain in descriptions or `x-bitcoin-*` extensions. More structured unit metadata and stronger constraints can be added in follow-up work.
Some RPCs return different types depending on argument values (e.g. verbosity levels). These are represented as `oneOf` in the result schema with free-text condition descriptions. This is accurate but not fully machine-parseable — a code generator cannot automatically determine which result variant corresponds to which argument value without parsing the description. I still we have enough information to satisfy humans an agents alike though.
### Regenerating the spec
The functional test invokes both RPCs, verifies valid JSON, checks public and hidden RPC handling, and covers representative generated schemas. It does not compare a committed generated artifact.
`getopenrpcinfo` omits hidden RPCs and arguments by default; `getopenrpcinfo(true)` includes them. The standard parameterless `rpc.discover` method returns the public document.
The RPC output documents which RPCs are available for any given built binary.
### Discussion questions
- Is this valuable/wanted?
- Do we like openrpc format? (less relevant if we don't want this in this repo, as another repo could generate one or many definitions).
- Should we cover "hidden" RPCs? They are currently hidden, but don't have to be...
My personal thoughts are that this is very nice to have.
ACKs for top commit:
dergoegge:
ACK ca9ffb8e12
achow101:
ACK ca9ffb8e12
sedited:
ACK ca9ffb8e12
w0xlt:
ACK ca9ffb8e12
Tree-SHA512: 5bf7abdb9f119d591306884f31b9da815908e5cb5812706c5496b39162c22cd949465d7b3ff1fdce7ff5b3c1b6367c14b6d28d58dee86698c0161d4d844d84c9
2cb3bfa8df scripted-diff: Use long form of shell options in Guix scripts (Hennadii Stepanov)
711eb10f08 guix: Add copyright headers to Guix scripts (Hennadii Stepanov)
80f831494e guix: Fix `glibc` version in comment (Hennadii Stepanov)
8916f7967e scripted-diff: Use C.UTF-8 locale in Guix scripts (Hennadii Stepanov)
Pull request description:
The C.UTF-8 locale is set by default in `guix shell`, and there is no reason to avoid it nowadays. This PR also silences superfluous warnings from Qt tools, making build logs cleaner and other issues easier to spot. For example:
```
Detected locale "C" with character encoding "ANSI_X3.4-1968", which is not UTF-8.
Qt depends on a UTF-8 locale, and has switched to "C.UTF-8" instead.
If this causes problems, reconfigure your locale. See the locale(1) manual
for more information.
```
Locales in the `guix-*` launch scripts have been updated as well for consistency with the rest of the codebase.
Additionally, the headers of the Guix scripts have been adjusted for [uniformity](https://github.com/bitcoin/bitcoin/pull/35775#discussion_r3636959268).
ACKs for top commit:
fanquake:
ACK 2cb3bfa8df
Tree-SHA512: 9e21d4ad50f5d583efdd8f79d9f96d45a92f1982ea3a422565bceea38aa50bbd9a9360972b37a1b49907e67523d68f3d1f889cb26179ce735453eb4fae4d3fa4
8a90c7cd97 guix: Build for macOS using LLVM toolchain only (Hennadii Stepanov)
7e1a750d45 guix, refactor: Use `target` variable instead of hardcoded value (Hennadii Stepanov)
Pull request description:
This PR makes macOS builds LLVM-only (non-GCC) by switching the build compiler in depends to `clang` + `libc++`.
See: https://github.com/bitcoin/bitcoin/issues/30206.
ACKs for top commit:
fanquake:
ACK 8a90c7cd97
Tree-SHA512: 02bfa784f89db980de2d2bb759200a0e8c0b10f48ac7e14e2bcdba75416aef30afa68d36e98c4652e18d282f023d1e94a351ff65b4a2731bdcac7c8a8c720623
a434d66025 cmake, translation: Specify English as target language explicitly (Hennadii Stepanov)
4097d6d968 cmake, translation: Sort messages within contexts alphabetically (Hennadii Stepanov)
312ab8ab0a cmake, translation: Skip source locations in TS files (Hennadii Stepanov)
4f553bd0da cmake, translation: Remove TS to XLIFF conversion (Hennadii Stepanov)
8c30055458 translation: Switch to Qt TS source file (Hennadii Stepanov)
Pull request description:
In Bitcoin Core v22.0, we [switched](https://github.com/bitcoin/bitcoin/pull/21694) from Qt TS to XLIFF translation source file to provide more context, specifically [developer notes](https://doc.qt.io/qt-6/i18n-source-translation.html#add-comments-for-translators), to translators on Transifex. That was very useful for translators back then, even though it required some extra complexity on our side.
Since then, Transifex has enabled support for developer notes in [Qt TS files](https://help.transifex.com/en/articles/6223301-qt-linguist) as well.
Therefore, I believe we should thank XLIFF for its service and retire it.
In addition to switching back to Qt TS, this PR introduces a few tweaks to the `lupdate` command (see the corresponding commit messages).
To summarize, this PR brings the following benefits:
1. Removal of obsolete code from the build system.
2. Minimal diffs during translation updates. For a recent example, see https://github.com/bitcoin-core/gui/pull/931. One can also apply the changes from bitcoin/bitcoin#34301 and run `cmake -B build --fresh -DBUILD_GUI=ON && cmake --build build -t translate` to observe the new minimal diff.
3. More stable string hashes on Transifex. They no longer include string `id`s, which makes this PR an alternative to https://github.com/bitcoin/bitcoin/pull/33270.
As a potential drawback, we are tying ourselves back to Qt's proprietary translation file format.
I've created an experimental resource on Transifex based on this branch: https://app.transifex.com/bitcoin/bitcoin/experimental-do-not-translate. Reviewers can use it to observe Transifex's support for the various features on the following messages:
- \# 11 - Developer Notes
- \# 144 - Plurals
- \# 510 - A disambiguation string (provided as a second argument to the [`tr()`](https://doc.qt.io/qt-6/qobject.html#tr) function) added to the string context.
ACKs for top commit:
l0rinc:
Code review ACK a434d66025
achow101:
ACK a434d66025
sedited:
ACK a434d66025
Tree-SHA512: 2f79af707974acd8c955e01c06b41794ae1702964bd5f6d260dba73f2f14d0b4b6e84f502f8515d84585e00db8db5b644cb6c47f91662a7ba5b6990f2d0ba115
efb4eae338 clusterlin: avoid recomputing intersections in MergeChunks (Pieter Wuille)
4b91ad149f clusterlin: reserve the suboptimal-chunk queue up front (Pieter Wuille)
e6ca996255 clusterlin: avoid heap allocations in GetLinearization (Pieter Wuille)
Pull request description:
These are a few minor and easy-to-review code changes to the SFL algorithm, which net a few % speedup (~2.6% speedup on `LinearizeOptimally*` benchmarks, ~5% on the `Historical` ones).
<details><summary>LLM benchmark results:</summary>
| Class | Ratio | 95% CI |
|-------------------|--------|------------------|
| HistoricalTotal | 0.9499 | [0.9312, 0.9709] |
| SyntheticTotal | 0.9760 | [0.9597, 0.9949] |
| HistoricalPerCost | 0.9545 | [0.9238, 0.9845] |
| SyntheticPerCost | 0.9934 | [0.9688, 1.0175] |
| **All (60)** | **0.9737** | **[0.9563, 0.9913]** |
Methology: One release-mode `bench_bitcoin` binary (GCC 15.2, `-O2`) was built for the base commit and one for the branch tip, and run as `bench_bitcoin -filter='.*LinearizeOptimally.*' -min-time=100` in 85 strictly alternating pairs of fresh process launches (170 launches, ~20 minutes total), each pinned to the same core of an otherwise idle Zen5 machine. For each of the 60 benchmarks, the per-launch ns/op values were averaged over each binary's 85 launches; the optimized/base ratios of these means were aggregated as a geometric mean per benchmark class, with 95% confidence intervals from 1000 bootstrap resamplings of the launches.
</details>
Disclosure: this code, comments, benchmarks, and selection of optimizations were done by Claude Fable 5. I reviewed the commits, and wrote the PR description.
ACKs for top commit:
optout21:
ACK efb4eae338
instagibbs:
light review ACK efb4eae338
marcofleon:
crACK efb4eae338
Tree-SHA512: 6bd523e8bc56dbc8ff2540ef26355e79895a8a11a26d08e22918aea0c002ee8f96916654d5a2b792515ef12ee022c65d71ab9a778f937a264aaf6e4ed6167b71
75929b11ed doc: add release note for submitSolution IPC changes (woltx)
ed75d70fdb refactor: centralize SubmitBlock result handling (w0xlt)
cbaa1696f3 mining: add reason and debug output to submitSolution (w0xlt)
83f3bc002d mining: clarify SubmitBlock result handling (w0xlt)
Pull request description:
`BlockTemplate.submitSolution` currently returns only a boolean, so IPC mining clients cannot determine why a submission failed without inspecting Bitcoin Core's debug log.
Returning `reason` and `debug`, as `Mining.submitBlock` already does, lets callers distinguish a concrete block rejection from a duplicate or inconclusive result. Here, `inconclusive` means the method returns failure, but validation did not determine that the submitted block is invalid.
This follow-up was suggested during the review of #34644:
https://github.com/bitcoin/bitcoin/pull/34644#discussion_r2853758006
This PR:
- Extracts a shared `SubmitBlock` helper that wraps `ProcessNewBlock` with `SubmitBlockStateCatcher` to capture `BlockValidationState`
- Adds `reason` and `debug` output parameters to `submitSolution`, matching `submitBlock`
- Makes both methods delegate to the same helper, eliminating duplicated logic
ACKs for top commit:
optout21:
ACK 75929b11ed
achow101:
light ACK 75929b11ed
Sjors:
ACK 75929b11ed
enirox001:
ACK 75929b11ed
sedited:
ACK 75929b11ed
Tree-SHA512: 31b1c305c20aaebdfa2d887665d9927830d0f97ba3c3469e2792148ad799d5a400a000cc0ca0b9add071d314e27c9da44d55228c442533a32a7c031678b78a55
7e19ce200b rpc: Fix descriptorprocesspsbt internal bug on invalid signatures (b-l-u-e)
Pull request description:
Fixes#32849
descriptorprocesspsbt crashes with an internal bug assertion when given a PSBT whose inputs looked “finalized” that is non-empty final_script_sig / witness but whose signatures did not verify like invalid Schnorr bytes with unusual sighash combinations such as SIGHASH_SINGLE | ANYONECANPAY.
Completeness was inferred with PSBTInputSigned, which only checks that final fields are present, not that they pass script verification. That let the RPC treat the PSBT as complete and call FinalizeAndExtractPSBT, where a failing invariant surfaced as CHECK_NONFATAL instead of a normal incomplete outcome.
This PR determines complete the same way as the wallet path: after ProcessPSBT, it recomputes PrecomputedTransactionData and sets complete only if every input passes PSBTInputSignedAndVerified.
Invalid signatures then yield complete: false and no hex, without going through the finalize path that asserted.
ACKs for top commit:
achow101:
ACK 7e19ce200b
rkrux:
lgtm re-ACK 7e19ce200b
Tree-SHA512: c534fb5bd3401ee3ac0bc27eaedaaaaf23e5e8510ed9b96a747bb0e41e2bd3d2031b6b08e933e139bb426e97bd05fab6e91397205f4be1db9d17630eea49a8bf
The C.UTF-8 locale is set by default in `guix shell`, and there is no
reason to avoid it nowadays. This change also silences superfluous
warnings from Qt tools, making build logs cleaner and other issues
easier to spot.
Locales in the `guix-*` launch scripts have been updated as well for
consistency with the rest of the codebase.
-BEGIN VERIFY SCRIPT-
sed -i "s/\<export LC_ALL=C\>/export LC_ALL=C.UTF-8/g" \
$( git grep -l "export LC_ALL=C" ./contrib/guix/* )
-END VERIFY SCRIPT-
fa0c8337a8 test: Nudge toward QT_STYLE_OVERRIDE=fusion on macOS (MarcoFalke)
faa50c08b1 refactor: Run clang-format on qt test_main.cpp (MarcoFalke)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/35771
Using the Fusion style works around QTBUG-49686.
Otherwise, there could be an invalid call under the QMacStyle.
Can be tested via:
```
QT_STYLE_OVERRIDE=macOS build/bin/test_bitcoin-qt # fails
QT_STYLE_OVERRIDE=fusion build/bin/test_bitcoin-qt # passes
```
ACKs for top commit:
hebasto:
ACK fa0c8337a8, tested on macOS Tahoe 26.5.2 (Intel).
Tree-SHA512: 3fd4e65f708ce5acd5243747661b76a5cf25e6810b49586dee5971f5f8bad13a907ee27d6830124c8c6d2b7e807661a59404f387ff59c1feeacfaa4b276bf270
075e7f4218 net: Simplify `AddressPosition` comparitor (rustaceanrob)
Pull request description:
Picked from #35713 because this appears unintentional. There were no cases where the source attempted to compare `AddressPosition` by const reference, but such a comparison is valid. This may be fixed by simplifying the comparison operator here, which also avoids copying the value.
Found in #35713:
```
<AddressPosition>' requested here
817 | BOOST_CHECK(addr_pos1 == addr_pos2);
| ^
/bitcoin-core/bitcoin/src/addrman.h:76:10: note: candidate function not viable: 'this' argument has type 'const AddressPosition', but method is not marked const
76 | bool operator==(AddressPosition other) {
```
ACKs for top commit:
maflcko:
lgtm ACK 075e7f4218
sedited:
Re-ACK 075e7f4218
Tree-SHA512: 4fc95026e6f9ec23757c97fc36a7fa94faf54a018727d0d2606215fc5705f3f1817607d8ff72544664cedc666914484ed490c6f75af63db1ce1a337be2e2949f
cf0f2aeae0 p2p: Assume v2transport for addresses from seeds (Martin Zumsande)
Pull request description:
gmaxwell noted in https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-5035178818 that addresses loaded from dns seeds and fixed seeds are still assumed to be v1, so a new node won't use `v2transport` for the first few connections it makes.
By now, the vast majority of reachable nodes (~80% according to https://bitnod.es/) in the network supports BIP324, and even if the optimistic guess would turn out to be wrong for a given node, we would just reconnect with v1.
This would also be necessary for a v2-only option (#30951), but I think it makes sense to change the default regardless of that PR.
Note that `-seednode` and `addr-fetch` connections already use `v2transport` by default.
ACKs for top commit:
w0xlt:
ACK cf0f2aeae0
sedited:
ACK cf0f2aeae0
willcl-ark:
ACK cf0f2aeae0
stratospher:
tested ACK cf0f2aea.
Tree-SHA512: 921dfcf56960f66c37346f0a4d07e8ad8aa617d4cd1c4db36474e8275c47d5eb70a34c32d1378cbcfce356348b12c36c8fedcdaee3c90c2c4963256857407db4
There were no cases where the source attempted to compare
`AddressPosition` by const reference, but such a comparison is valid.
This may be fixed by simplifying the comparison operator here, which
also avoids copying the value.
Found in #35713:
```
<AddressPosition>' requested here
817 | BOOST_CHECK(addr_pos1 == addr_pos2);
| ^
/bitcoin-core/bitcoin/src/addrman.h:76:10: note: candidate function not viable: 'this' argument has type 'const AddressPosition', but method is not marked const
76 | bool operator==(AddressPosition other) {
```
226e6388b7 depends: Update Qt to 6.8.4 (Hennadii Stepanov)
Pull request description:
Release notes: https://code.qt.io/cgit/qt/qtreleasenotes.git/about/qt/6.8.4/release-note.md
`depends/patches/qt/qtbase_platformsupport.patch` has been dropped as the fix was backported upstream in 5afcc64fd7.
ACKs for top commit:
fanquake:
ACK 226e6388b7
Tree-SHA512: 20f371384c1fcfd5018218ef262b10568bd009e2f1fafca9fa42788aca4683ec706b7a4e35816b884b294b5f9bdb1aefda0bad75921d9e426558cccd80f913cd
51d36dfd07 qt: Fix `-Wsfinae-incomplete` warnings when building with GCC 16.x (Hennadii Stepanov)
Pull request description:
According to the CMake documentation for [`AUTOMOC`](https://cmake.org/cmake/help/latest/prop_tgt/AUTOMOC.html), all `moc` output files that are not included in a source file are aggregated into the CMake-generated `<AUTOGEN_BUILD_DIR>/mocs_compilation.cpp`, which is added to the target's sources.
Within that single translation unit, `moc`-generated code checks the completeness of a signal or slot parameter type while it is still only forward-declared, and the type is completed later, when a subsequently included `moc_*.cpp` file pulls in the header that defines it. GCC 16.x diagnoses this pattern with the `-Wsfinae-incomplete` warning, which is enabled by default.
Including the `moc` output files at the end of the corresponding source files excludes them from `mocs_compilation.cpp`, so each one is compiled in a translation unit where the relevant types are complete.
FWIW, Qt itself uses the same approach throughout its own codebase. Also see https://www.youtube.com/watch?v=Cx_m-qVnEjo.
---
Steps to reproduce on the master branch @ 18c05d9301 on Fedora 44 (GCC 16.1.1):
```console
$ cmake --preset dev-mode
$ cmake --build build_dev_mode -t bitcoind
$ cmake --build build_dev_mode -t bitcoin-qt
[166/172] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/mocs_compilation.cpp.o
In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_bitcoingui.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:9:
/home/hebasto/dev/bitcoin-gui/src/qt/bitcoingui.h:67:7: warning: defining ‘BitcoinGUI’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
67 | class BitcoinGUI : public QMainWindow
| ^~~~~~~~~~
In file included from /usr/include/qt6/QtCore/qobject.h:19,
from /usr/include/qt6/QtWidgets/qwidget.h:10,
from /usr/include/qt6/QtWidgets/qdialog.h:9,
from /usr/include/qt6/QtWidgets/QDialog:1,
from /home/hebasto/dev/bitcoin-gui/src/qt/addressbookpage.h:8,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_addressbookpage.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:2:
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
In file included from /home/hebasto/dev/bitcoin-gui/src/qt/paymentserver.h:35,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_paymentserver.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:27:
/home/hebasto/dev/bitcoin-gui/src/qt/sendcoinsrecipient.h:15:7: warning: defining ‘SendCoinsRecipient’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
15 | class SendCoinsRecipient
| ^~~~~~~~~~~~~~~~~~
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
In file included from /home/hebasto/dev/bitcoin-gui/src/qt/psbtoperationsdialog.h:13,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_psbtoperationsdialog.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:30:
/home/hebasto/dev/bitcoin-gui/src/qt/walletmodel.h:48:7: warning: defining ‘WalletModel’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
48 | class WalletModel : public QObject
| ^~~~~~~~~~~
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_qvalidatedlineedit.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:32:
/home/hebasto/dev/bitcoin-gui/src/qt/qvalidatedlineedit.h:13:7: warning: defining ‘QValidatedLineEdit’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
13 | class QValidatedLineEdit : public QLineEdit
| ^~~~~~~~~~~~~~~~~~
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_rpcconsole.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:37:
/home/hebasto/dev/bitcoin-gui/src/qt/rpcconsole.h:43:7: warning: defining ‘RPCConsole’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
43 | class RPCConsole: public QWidget
| ^~~~~~~~~~
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_sendcoinsentry.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:39:
/home/hebasto/dev/bitcoin-gui/src/qt/sendcoinsentry.h:26:7: warning: defining ‘SendCoinsEntry’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
26 | class SendCoinsEntry : public QWidget
| ^~~~~~~~~~~~~~
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
[172/172] Linking CXX executable bin/bitcoin-qt
```
ACKs for top commit:
maflcko:
review ACK 51d36dfd07🦅
Tree-SHA512: fe48e4925aaccb833bc065aa7f988f22482ef83d3d555e601d3955109f840b221be971c560668eac79506f567999d4e1b2a464ec3717b5cecb9a3eaaa1dbc01b
7298281ba8 bitcoin-util: replace netmagic command with getchainparams command (Anthony Towns)
Pull request description:
This is a follow-up to #35610. It replaces the `netmagic` command with a more versatile `getchainparams` command, as suggested in https://github.com/bitcoin/bitcoin/pull/35610#issuecomment-4974474640.
ACKs for top commit:
maflcko:
re-ACK 7298281ba8📓
ajtowns:
Coauthor ACK 7298281ba8
sedited:
ACK 7298281ba8
Tree-SHA512: 153e97eb8d6bc6d98d925ce9718239c6ff5d7a80b760e7f3b807fc36f78e8da8550033c31016e1d1aee11484392dd80b76f9b1496a8dc6e47825910c170a0cd1
faada35f9c fuzz: [refactor] Use 100'000 digit separator in __AFL_LOOP (MarcoFalke)
fae067ec4a fuzz: Avoid dangling prevoutfetch threads after AFL fork (MarcoFalke)
Pull request description:
Presumably fixes https://issues.oss-fuzz.com/issues/536943806
This is a bit confusing, because the issue was already fixed in commit f608a409f7, by removing the AFL forkserver.
However, OSS-Fuzz doesn't go through the AFL_LOOP, but through the AFL libFuzzer driver:
```
#0 0x7e055245baab in __pthread_clockjoin_ex /build/glibc-B3wQXB/glibc-2.31/nptl/pthread_join_common.c:89:6
#1 0x5a27e904dcdd in operator() /src/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:293:14
#2 0x5a27e904dcdd in Join<(lambda at /src/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:292:44)> /src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_thread_arg_retval.h:75:9
#3 0x5a27e904dcdd in ___interceptor_pthread_join /src/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:292:25
#4 0x5a27e90f4044 in std::__1::thread::join()
#5 0x5a27e9366277 in ThreadPool::Stop() [bitcoin-core/src/util/threadpool.h:146](7d8137c141/src/util/threadpool.h (L146)):53
#6 0x5a27e9365db9 in ThreadPool::~ThreadPool() [bitcoin-core/src/util/threadpool.h:94](7d8137c141/src/util/threadpool.h (L94)):9
...
#32 0x5a27e95a9506 in (anonymous namespace)::ResetChainman(TestingSetup&) (.12669) [bitcoin-core/src/test/fuzz/process_messages.cpp:44](7d8137c141/src/test/fuzz/process_messages.cpp (L44)):27
#33 0x5a27e95a8c60 in process_messages_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>) [bitcoin-core/src/test/fuzz/process_messages.cpp:141](7d8137c141/src/test/fuzz/process_messages.cpp (L141)):9
...
#36 0x5a27e97b7190 in test_one_input(std::__1::span<unsigned char const, 18446744073709551615ul>) bitcoin-core/src/test/fuzz/fuzz.cpp:86:5
#37 0x5a27e97b7190 in LLVMFuzzerTestOneInput bitcoin-core/src/test/fuzz/fuzz.cpp:214:5
#38 0x5a27e90ada19 in LLVMFuzzerRunDriver /src/aflplusplus/utils/aflpp_driver/aflpp_driver.c:427:13
#39 0x5a27e90ad69b in main /src/aflplusplus/utils/aflpp_driver/aflpp_driver.c:323:10
#40 0x7e055223b082 in __libc_start_main /build/glibc-B3wQXB/glibc-2.31/csu/libc-start.c:308:16
#41 0x5a27e8fc602d in _start
```
So the correct fix would be to set `AFL_DRIVER_DONT_DEFER=1`. Ref: ad5304010a/utils/aflpp_driver/aflpp_driver.c (L161)
However, I don't know how to do this on OSS-Fuzz, so just drop the threads for now, because there are dedicated fuzz targets to test the multi-threaded case anyway.
ACKs for top commit:
l0rinc:
ACK faada35f9c
andrewtoth:
lgtm ACK faada35f9c
sedited:
ACK faada35f9c
Tree-SHA512: c249d7267f789084968f8510531f60fc71c9fbd6b4e574a181fd6da1af19a3cb7c03ba60c3ca70244b56ce44c602293517cdb6b0e969b4a2cc9d1afaa49ab0f8
f3f302150b ci: Put space and non-ASCII char in `BASE_BUILD_DIR` (Hennadii Stepanov)
a7e980af31 build: Quote host paths in NSIS installer template (Hennadii Stepanov)
Pull request description:
The CI scratch directory contains a space and non-ASCII symbols to test path handling (see #34614). However, the GHA workflows [override](18c05d9301/.github/actions/configure-environment/action.yml (L10)) `BASE_BUILD_DIR` to `${{ runner.temp }}/build` via the `configure-environment` action, bypassing the `$BASE_SCRATCH_DIR/build-$HOST` [default](18c05d9301/ci/test/03_test_script.sh (L109-L110)) from `03_test_script.sh`.
The first commit fixes the NSIS template, which otherwise breaks the `deploy` target when paths contain spaces.
Related to #35356.
ACKs for top commit:
maflcko:
re-ACK f3f302150b🥘
l0rinc:
code review ACK f3f302150b
Tree-SHA512: 5785dc13961d656e73759888d653967e43d6a9e1eec46a12301c65d042bc53a824f2dd9cdaa602c1bfb1afb52c4753099a705eab3a337f88ac6a68893e2de7f9
3bfdcbd7ee coins: reuse cache hasher for txid set (Lőrinc)
2beab94896 coins: use SipHash-1-3-UJ for `CCoinsMap` (Lőrinc)
7ff55cc650 bench: add fixed-width SipHash benchmarks (Lőrinc)
3aea85411f test: add SipHash-1-3-UJ coverage (Pieter Wuille)
a0ccd4ad17 crypto: add fixed-width SipHash-1-3-UJ (Pieter Wuille)
c2d7931b5c crypto: add generic SipHash-1-3-UJ (Pieter Wuille)
25bfca06d6 refactor: simplify adding SipHash-1-3-UJ (Lőrinc)
af50ba8500 test: add shared SipHash vectors (Lőrinc)
Pull request description:
**Problem:** The in-memory UTXO cache hashes `COutPoint` keys containing a 32-byte txid and a 32-bit output index.
SipHash-2-4 processes the txid as four independent 64-bit blocks, so its optimized 32-byte and 36-byte paths both take 14 SipRounds.
This also matters for hash-prefix index work such as [#35531](https://github.com/bitcoin/bitcoin/pull/35531): once a persisted key format chooses a hash function, changing it later requires reindexing.
**Fix:** Add `SipHasher13UJ`, a custom block-oriented variant combining Pieter Wuille's jumbo-block suggestion with SipHash-1-3, the reduced-round variant discussed in the [SipHash analysis](https://eprint.iacr.org/2012/351.pdf).
It provides inline `Hash` overloads for the fixed-width inputs used here.
Use a dedicated `SaltedCoinsCacheHasher` for `CCoinsMap` and `CoinsViewOverlay`'s temporary earlier-txid set, while other outpoint tables remain on SipHash-2-4.
The salted hash values vary between restarts and are never persisted or sent over the network.
**Design:** `SipHasher13UJ` accepts normal 64-bit blocks and 256-bit jumbo blocks.
For hash-table use, cryptographic hash outputs must make up all but a small bounded number of retained jumbo blocks.
The construction mixes all four limbs around one SipRound, omits byte-oriented padding, and uses an `"unpadded"` finalizer distinct from standard SipHash-1-3.
The fixed-width paths take four rounds for one `uint256` jumbo block and five when followed by one normal block.
For outpoints, the 32-bit output index is zero-extended into a normal 64-bit block.
Retained `CCoinsMap` entries identify real transaction outputs, so their keys contain computed txids.
Missing-input validation may probe arbitrary claimed prevouts, but `FetchCoin()` immediately erases their temporary entries when the backend lookup fails, so non-hash keys cannot accumulate.
The assumeutxo loader assumes snapshot txids are valid while loading and verifies the complete snapshot's content hash before activation.
Every entry in the temporary earlier-txid set is a computed transaction hash, and the set is bounded by the block's transaction count.
This construction is limited to local hash tables and is not a general-purpose or protocol SipHash replacement.
Pieter discussed the construction with [SipHash co-author Jean-Philippe Aumasson](https://github.com/bitcoin/bitcoin/pull/35215#issuecomment-4385336928), whose preliminary analysis did not find an easier collision construction and supported SipHash-1-3 for this hash-table use.
<img width="2100" height="860" alt="siphash_compare_updated" src="https://github.com/user-attachments/assets/cefec6f8-5ec0-450a-a0a2-f946de9ef36d" />
**Structure:** Shared vectors first cover the existing generic and fixed SipHash-2-4 paths in C++, the generic path in Python, and their randomized equivalence in the fuzzer.
A behavior-neutral refactor then moves the round, compression, and finalization logic into inline `SipHashState` methods; assembly inspection shows that the fixed-width paths retain their instruction counts, while the generic byte loop retains its prior code generation through a local state copy.
Three Pieter-authored commits add the generic UJ specification, fixed-width implementation, and shared correctness coverage.
Benchmarks follow that coverage, then separate commits change `CCoinsMap`'s hasher and reuse it for the temporary earlier-txid set.
**Tests:** The shared JSON supplies the same byte sequences to the generic C++ and Python SipHash-2-4 implementations, with applicable fixed-width paths checked against the same expected output.
The SipHash-2-4 rows include the 64 official vectors for inputs from 0 to 63 bytes and cases that vary input chunking.
The UJ outputs were generated by an independent implementation and are checked using normal blocks, equivalent zero-extended jumbo blocks, and applicable fixed-width `Hash` overloads.
The integer fuzzer extends these comparisons to arbitrary values and mixed normal/jumbo block encodings.
[Counting the dbcache buckets](https://gist.github.com/l0rinc/d68f56c3ed89f76f56da6632ef6f2d92) indicates the new outpoint hasher retains the uniform bucket distribution expected by `CCoinsMap`:
<img width="1200" height="750" alt="ccoinsmap-collisions" src="https://github.com/user-attachments/assets/eeedec81-acdc-4adf-a9c8-bfce089700da" />
**Benchmarks:** Fixed-width microbenchmarks compare SipHash-2-4 with SipHash-1-3-UJ for 32-byte hashes and inputs containing a 32-byte hash plus a 32-bit index.
Reported aarch64 measurements and an [independent x86_64 run](https://github.com/bitcoin/bitcoin/pull/35215#issuecomment-4400609637) show the outpoint path is about 2x faster.
<details><summary>Benchmark runner</summary>
```bash
for COMPILER in gcc clang; do \
if [ "$COMPILER" = gcc ]; then CC=gcc; CXX=g++; else CC=clang; CXX=clang++; fi; \
cmake -B "build-bench-$COMPILER" -DCMAKE_BUILD_TYPE=Release -DBUILD_BENCH=ON -DBUILD_TESTS=OFF -DBUILD_GUI=OFF -DENABLE_WALLET=OFF -DCMAKE_C_COMPILER="$CC" -DCMAKE_CXX_COMPILER="$CXX" >/dev/null 2>&1 && \
cmake --build "build-bench-$COMPILER" --target bench_bitcoin -j"$(nproc)" >/dev/null 2>&1 && \
echo "" && echo "$(date -I) | SipHash fixed-width microbench | $("$CXX" --version | head -1) | $(hostname) | $(uname -m) | $(lscpu | awk -F: '/Model name/{print $2; exit}' | xargs) | $(nproc) cores | $(free -h | awk '/^Mem:/{print $2}') RAM" && \
"build-bench-$COMPILER/bin/bench_bitcoin" -filter='SipHash.*32b|SipHash.*36b' -min-time=10000; \
done
```
</details>
A two-run GCC `-reindex-chainstate` comparison of the same `CCoinsMap` hot path through height 957,759 with `-dbcache=2000` on a Ryzen 7 3700X/SSD reduced mean wall time from 11,278 s to 10,759 s, a ~5% validation speedup.
ACKs for top commit:
achow101:
light ACK 3bfdcbd7ee
sipa:
ACK 3bfdcbd7ee (to the extent the code/ideas aren't my own)
andrewtoth:
ACK 3bfdcbd7ee
optout21:
ACK 3bfdcbd7ee
Tree-SHA512: c3c66051cb1ebdb0cddbc8b8bed2297c524842f92960531de23d9586c5bb950b302c33d06fc15c2320cbbf97273b22ec3f17fd0e7ddcc7df4a8132a47a606277
6aa5d8d948 blockencodings: fix extra transaction count (Lőrinc)
be4e64d9e4 test: characterize extra transaction miscount (Lőrinc)
Pull request description:
A short ID collision can invalidate a mempool-sourced transaction after an unrelated transaction was found in extra_txn.
Track each slot's source so extra_count is decremented only when the invalidated slot came from extra_txn. Retain the source after a collision to preserve the rule that later candidates do not refill the slot.
ACKs for top commit:
l0rinc:
retested ACK 6aa5d8d948
andrewtoth:
ACK 6aa5d8d948
sedited:
ACK 6aa5d8d948
Tree-SHA512: d4407dca7ca2b46795e52a5d611b66072d75ba966416d4ede27fa776ac0a7cbde52c8abae2daec532ec5f24d88e608622168ca066c0d1dbc133e098c8e6fe85a
The GHA workflows override `BASE_BUILD_DIR`, so the build tree no longer
lives under `BASE_SCRATCH_DIR` and its word-splitting and UTF-8 coverage
is bypassed on CI. Restore it by putting a space and a non-ASCII symbol
in the externally defined path as well.
By now, the vast majority of nodes in the network supports BIP324.
Even if the optimistic guess would turn out to be wrong for a given
node, we would just reconnect with v1.
This is better than making v1 connections with peers when both nodes support v2.
