aa01721c89 test: add interface_gui.py to test bitcoin-gui startup via RPC (Ryan Ofsky)
Pull request description:
Adds a functional test that starts bitcoin-qt using QT_QPA_PLATFORM=minimal for headless operation, then verifies it responds to a stop RPC call. This detects startup crashes in the GUI that have no CI coverage today like https://github.com/bitcoin-core/gui/issues/940
The new test is currently skipped on macos and windows due to different problems on those platforms that may be resolved with future PRs. Fixing the windows issue should also allow the `tool_bitcoin.py` test to be enabled on windows, and fixing the macos issue should allow Qt addressbook and wallet tests to be enabled on macos.
ACKs for top commit:
achow101:
ACK aa01721c89
sedited:
ACK aa01721c89
pablomartin4btc:
ACK aa01721c89
hebasto:
ACK aa01721c89.
Tree-SHA512: 84873aed41a856322eca1c391d3ff19b6eb4a0aa253d09ace342e3efe970a330cb11506eb4efe2580b7991132b644c7408feb013c18dbfbfaaf879f88f12e02e
683ae4c520 guix: consolidate config flags (fanquake)
665f11d04a guix: consolidate gcc toolchain setup (fanquake)
288f76ed0f guix: consolidate mingw-w64 toolchain setup (fanquake)
cc9b0f2266 guix: consolidate LLVM toolchain setup (fanquake)
b12a70f330 guix: turn linux/win linker warnings into errors (fanquake)
Pull request description:
This deduplicates setup code, as well as adds flags to turn linker warnings into errors, which is easier now that the GUI build has been split out (the gui link warns about shared libs during linking).
ACKs for top commit:
hebasto:
ACK 683ae4c520, I have reviewed the code and it looks OK.
Tree-SHA512: 08b4fa14494481149844750bd6741c61b3a9367eed46923e8c788f3cd22ea9c4b319326075df0114dbb17e11f0dd1340999ba80e40a244208e6b4e90c86e3361
Can do this now that the GUI has been split out.
riscv64-linux-gnu failus due to
https://github.com/boostorg/test/issues/345:
```bash
[102%] Linking CXX executable ../../bin/test_bitcoin
/gnu/store/r03804zpq5i6wsalx0yaqrr5jb7pqrmv-binutils-cross-riscv64-linux-gnu-2.46.0/bin/riscv64-linux-gnu-ld: CMakeFiles/test_bitcoin.dir/main.cpp.o: in function `boost::fpe::disable(unsigned int)':
/bitcoin/depends/riscv64-linux-gnu/boost/include/boost/test/impl/execution_monitor.ipp:1538:(.text+0x9dc8): warning: fedisableexcept is not implemented and will always fail
/gnu/store/r03804zpq5i6wsalx0yaqrr5jb7pqrmv-binutils-cross-riscv64-linux-gnu-2.46.0/bin/riscv64-linux-gnu-ld: CMakeFiles/test_bitcoin.dir/main.cpp.o: in function `boost::fpe::enable(unsigned int)':
/bitcoin/depends/riscv64-linux-gnu/boost/include/boost/test/impl/execution_monitor.ipp:1502:(.text+0x9d76): warning: feenableexcept is not implemented and will always fail
collect2: error: ld returned 1 exit status
```
Darwin could be done after something like
https://github.com/bitcoin/bitcoin/pull/35756.
c9a70f9338 script: qa: Improve Key::Fingerprint type safety (David Gumberg)
Pull request description:
Extracted from pseudoramdom's work in #35436:
Instead of using c style arrays for key fingerprints, use `std::array`'s whose length can always reasoned about at compile time and for most operations the compiler enforces the size being correct.
```cpp
using KeyFingerprint = std::array<unsigned char, 4>;
```
```diff
- unsigned char vchFingerprint[4];
+ KeyFingerprint fingerprint;
```
This allows the replacement of a lot of raw `memcpy` + trust-me-bro lengths, with the assignment operator:
```cpp
- memcpy(ret.vchFingerprint, vchFingerprint, 4);
+ ret.fingerprint = fingerprint;
```
This commit also adds two helper functions for
- Retrieving the [fingerprint of a key identifier](https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#user-content-Key_identifiers) (`CKeyID`)
- Retrieving the fingerprint of the key identifier of an XPUB.
ACKs for top commit:
w0xlt:
ACK c9a70f9338
sedited:
ACK c9a70f9338
pseudoramdom:
Code review ACK w/ some minor nits c9a70f9338
polespinasa:
ACK c9a70f9338
Tree-SHA512: 3ee76742c0bc317dfbc12a6731afdcc40495db6e4d5d94880d0a721990d36cb3e4d374ccc96079ba1f8ad3f88581ee5a609bfe259c0ea7cd28cade373aac1b38
d3d74e701f ipc, refactor: Update mp::g_thread_context references (Ryan Ofsky)
2d3f72fd3f ipc, refactor: Update mp::SpawnProcess call (Ryan Ofsky)
e9f19815ca ipc, refactor: Add Stream type alias and use it (Ryan Ofsky)
3859805f05 ipc, refactor: Add SocketId type alias and use it (Ryan Ofsky)
2ee9b69c7a ipc, refactor: Add ProcessId type alias and use it (Ryan Ofsky)
3449797141 ipc: Avoid 'unistd.h' error with MSVC (Ryan Ofsky)
dbcc192dce ipc, refactor: fix include order (Ryan Ofsky)
7c86d4834e ipc, refactor: use native path separators in test (Ryan Ofsky)
00287b9a34 ipc, refactor: Change Protocol class field order (Ryan Ofsky)
33d37f3c35 ipc, refactor: Drop connect/listen/serve exe_name parameters (Ryan Ofsky)
794940469e ipc, moveonly: combine ipc_test.cpp and ipc_tests.cpp (Ryan Ofsky)
Pull request description:
This PR makes Bitcoin Core changes needed to be compatible with https://github.com/bitcoin-core/libmultiprocess/pull/274, which changes the libmultiprocess API to stop using unix-specific types so it is compatible with windows. (Windows support is added in followups: https://github.com/bitcoin-core/libmultiprocess/pull/231 and https://github.com/bitcoin/bitcoin/pull/32387.)
The PR uses some [compatibility shims](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-wins/src/ipc/util.h) so it can be reviewed and merged without needing to merge https://github.com/bitcoin-core/libmultiprocess/pull/274 first and bump the libmultiprocess subtree. These can be deleted when the subtree is updated.
---
Review note: All the changes here are refactoring, and you don't really need to know anything about IPC or Windows to review this code. It is also a mostly move-only change (131 lines added, 96 removed, 215 moved)
ACKs for top commit:
xyzconstant:
tACK d3d74e701f
enirox001:
ACK d3d74e701f
Sjors:
ACK d3d74e701f
ViniciusCestarii:
re-ACK d3d74e701f tested locally on Linux
Tree-SHA512: cd48708f9fd086ac8127dc75cfaf4bd8f8da81e07d11b2c9e65fd9061ffa33478bffc6fd6fa4b3505e86c6437752578fe6e5bd590c683c3bc9969093103a5608
bc7d905046 addrman: remove unreachable tried-collision branch (Bruno Garcia)
Pull request description:
`ResolveCollisions_()` had a fallback for the case where a pending tried collision no longer collided because the destination tried slot became empty.
Under current addrman invariants this cannot happen: once an entry is added to `m_tried_collisions`, the corresponding tried slot remains occupied until the collision is resolved. The only other valid outcomes are that the pending new entry disappears or becomes invalid, both of which are already handled.
Remove the dead branch and replace the implicit assumption with assertions in `ResolveCollisions_()` and `SelectTriedCollision_()`.
It came to my mind when taking a look at the fuzz coverage report for the addrman harness. After years (?) of fuzzing, I was trying to understand if it was a fault on the harness or a dead branch.
ACKs for top commit:
Herb-ops:
ACK bc7d905046
danielabrozzoni:
tACK bc7d905046
stratospher:
ACK bc7d905. `MakeTried` is the only place where we clear the tried table slot but we also refill the same slot here under the cs hold. so makes sense that during a node's runtime destination tried slot which has a previous entry/collison can't be empty (unless some id internal corruption).
naiyoma:
ACK bc7d905046
mzumsande:
Code Review ACK bc7d905046
Tree-SHA512: 57236c0f95ec1028e831aeffdc0639faf5fe083b108cb2869fa1ce237bb37ac3be96d6a49d9a6a8f4a21558d9dcfa015ab3276cc30a8826e5c65e8b0853acd45
a92e93429e guix: Drop unused `(guix licenses)` import from `manifest_build.scm` (Hennadii Stepanov)
Pull request description:
This was overlooked in bitcoin/bitcoin#34948.
ACKs for top commit:
fanquake:
ACK a92e93429e
Tree-SHA512: ab1ff63b49da104b21c4731ea06d3a4db009712be7c50f907cd46d0ad2936f7a236915bbe436801f02ea32bd781dfb5f38d349dda0a99a563701bef1089f35a6
baa5a2ce43 guix: pass --disable-tm-clone-registry to base GCC (fanquake)
9c2589630f guix: mirror some arguments from linux-gcc to mingw-w64-gcc (fanquake)
e0b8fbde89 guix: disable-nls in *-base-gcc (fanquake)
7dc87f8e1e guix: disable-lto in *-base-gcc (fanquake)
9ed3d6ef2a guix: modernise style in *-base-gcc (fanquake)
Pull request description:
We don't use LTO in release builds, and neither do any of our dependencies, so disable support in GCC (`--disable-lto`):
> Enable support for link-time optimization (LTO). This is enabled by default, and may be disabled using --disable-lto.
This reduces what needs to be compiled when building the Guix toolchain. It would also make it clear if something started using it.
Also disable Native Language Support (`--disable-nls`):
> The --enable-nls option enables Native Language Support (NLS), which lets GCC output diagnostics in languages other than American English. Native Language Support is enabled by default if not doing a canadian cross build.
Also disable support for transactional memory (`--disable-tm-clone-registry`):
> Disable TM clone registry in libgcc. It is enabled in libgcc by default. This option helps to reduce code size for embedded targets which do not use transactional memory.
See https://gcc.gnu.org/install/configure.html.
ACKs for top commit:
hebasto:
ACK baa5a2ce43, I have reviewed the code and it looks OK.
Tree-SHA512: 6fccab4c43f5c506c51ecd0e8c903ca82125a5ade5008cef7c0e50667ec7a417803ad8c26e670f5718cfbca125f6c32fd6534f945adf58f13ea2fc23b16244ac
f5d7cc66ec doc: Discourage adding AI agents as commit authors (sedited)
Pull request description:
The goal of the AI policy is to ensure that contributors maintain the responsibility of understanding the change they are contributing. Adding AI agents as co-authors undermines this. I believe this philosophy should extend to commit co-authors in general: They should only be added if they themselves are capable of fully understanding the commit.
This contribution was sparked by maflcko's comment here: https://github.com/bitcoin/bitcoin/pull/35551#pullrequestreview-4642682025 .
ACKs for top commit:
l0rinc:
ACK f5d7cc66ec
yancyribbens:
ACK f5d7cc66ec
xyzconstant:
ACK f5d7cc66ec
jonatack:
ACK f5d7cc66ec modulo IANAL, IDK if there are copyright issues with using/crediting work by LLM agents
w0xlt:
ACK f5d7cc66ec
pablomartin4btc:
ACK f5d7cc66ec
theStack:
ACK f5d7cc66ec
Tree-SHA512: 134902fcf4748bf991a6c3df6e41d5edbbc0e57c35d15d9d84fe4d30153549c05546ea5fe22226356d01f6df15f0c7177d9b1af62b9f90982a7788613bdd94a9
349c72ee00 net_processing: Drop unnecessary txid arg from InitiateTxBroadcastToAll (Anthony Towns)
12b0dc33c4 doc: Add release note for -txsendrate etc (Anthony Towns)
5cde66341a tests: basic functional test for tx rate limiting (Anthony Towns)
4842903ac1 rpc: report -txsendrate and bucket info via getnetworkinfo (Anthony Towns)
74a47a5207 init: add -txsendrate configuration parameter (Anthony Towns)
6307bd034b net_processing: Provide a 30bpm heartbeat log while inv backlog is in use (Anthony Towns)
df31ee57aa net_processing: add a global delay queue for sending txs (Anthony Towns)
7927650e56 util/tokenbucket.h: Provide a generic TokenBucket class (Anthony Towns)
749bb447f8 txmempool: Drop CompareMiningScoreWithTopology (Anthony Towns)
e1b7490fbc net_processing: Replace CompareInvMempoolOrder (Anthony Towns)
6cfc65d210 txmempool: Add ExtractBestByMiningScoreWithTopology (Anthony Towns)
026f70e05f net_processing: Remove per-peer rate-limiting (Anthony Towns)
46c8c471dc net_processing: bump last_inv_sequence for bip35 messages explicitly (Anthony Towns)
Pull request description:
Per-peer `m_tx_inventory_to_send` queues have CPU and memory costs that scale with both queue size and peer count. Under high transaction volume, this has previously caused severe issues ([May 2023 disclosure][1]) and still can cause measurable delays ([Feb 2026 Runestone surge][2], with the msghand thread observed hitting 100% CPU and queue memory reaching ~95MB).
This PR replaces the per-peer rate limiting with a global queue using dual token buckets (limiting transaction by both count and serialized size). Transactions that arrive within the bucket capacity still relay nearly immediately, but excess transactions queue in a global backlog and drain as the token buckets refill.
Key parameters:
- Count bucket: 14 tx/s, 420 capacity (30s buffer)
- Size bucket: 20 kB/s (~12 MB/600s), 50 MB capacity
- Outbound peers refill faster by a factor of 2.5
Per-peer queues are retained solely for privacy batching and are always fully emptied, removing the old `INVENTORY_BROADCAST_MAX` cap.
This reduces the memory and CPU burden during transaction spikes when the queuing logic is engaged from O(queue * peers) to O(queue), as the queued transactions no longer need to be retained per-peer or re-sorted per-peer.
Design discussion: https://gist.github.com/ajtowns/d61bea974a07190fa6c6c8eaef3638b9
[1]: https://bitcoincore.org/en/2024/10/08/disclose-large-inv-to-send/
[2]: https://bnoc.xyz/t/increased-b-msghand-thread-utilization-due-to-runestone-transactions-on-2026-02-17/81
ACKs for top commit:
sipa:
Code review ACK 349c72ee00. I haven't tested it myself yet (though switched my well-connected node to it now), but the posted benchmarks and analyses look convincing.
instagibbs:
reACK 349c72ee00
mzumsande:
ACK 349c72ee00
Tree-SHA512: 2196a23308cb7fe36738cf638edf5c5b0e9ba32b11c083609fd8b50291e05bb33484f9921f8beab28d94c58d1adddea4c8ae1182a60a7f53f54be7370e2a0e47
29b124416e doc: add release notes for 32800 (Musa Haruna)
5d25a0c28d rpc: add `vsize_adjusted` field to getrawtransaction output for mempool transactions (Musa Haruna)
eaef8d3111 rpc: add `vsize_adjusted` and `vsize_bip141` field to mempool-related RPCs (Musa Haruna)
Pull request description:
### Motivation and Problem
`CTxMemPoolEntry::GetTxSize()` returns the larger of two values: the BIP 141 virtual size (vsize) and the "sigop-adjusted size." This sigop-adjusted size is used by mempool validation and mining algorithms as a safeguard to prevent overfilling blocks with transactions that approach both the weight and signature operation (sigop) limits in a way that could harm block space efficiency.
In the current implementation, the sigop-adjusted size is reported as the "vsize" in RPCs that provide mempool transaction data, such as `getmempoolentry`, `getrawmempool`, `testmempoolaccept`, and `submitpackage`. However, the documentation for these RPCs typically describes this value simply as the "virtual transaction size as defined in BIP 141," without acknowledging the sigop adjustment. Since the reported size may differ from the pure BIP 141 definition, this confuses people as in this [tweet](https://x.com/mononautical/status/1646166180145577990?s=20), discrepancy can be misleading, as the reported size may differ from the pure BIP 141 definition.
### Proposed Solution
To resolve this, all mempool-related RPCs now return two separate fields:
**vsize_adjusted:** the sigop-adjusted size, i.e. max(BIP 141 vsize, sigop-adjusted size), which reflects the value previously returned under the vsize label and continues to drive mempool acceptance and block template scoring.
**vsize_bip141:** the pure BIP 141 virtual size, strictly `ceil(weight/4)`, matching the consensus definition is now reported here in `vsize_bip141` field. `vsize` field in now marked as DEPRECATED and users are advised to use the new `vsize_bip141` field for pure virtual size instead.
This means that clients that depends on mempool policy size reported vsize will use `vsize_adjusted`, while `vsize` is now purely BIP 141.
Additionally, this PR updates the relevant RPC help text to clearly document the distinction between these two sizes, and adds supporting documentation `doc/policy/feerates-and-vsize.md` to better explain fee rates, virtual size calculations, sigop adjustments, and the mempool policy heuristics.
A new field, vsize_adjusted, has also been added to the getrawtransaction RPC result when input information (transaction is in the mempool) is available. Exposing this value provides users with more precise insight into how the transaction’s sigops impact its effective size for policy and fee estimation.
Note: This picks up work from the closed [#27591](https://github.com/bitcoin/bitcoin/pull/27591)
Fixes [#32775](https://github.com/bitcoin/bitcoin/issues/32775)
ACKs for top commit:
achow101:
ACK 29b124416e
hodlinator:
re-ACK 29b124416e
ismaelsadeeq:
Code review ACK 29b124416e
sedited:
ACK 29b124416e
Tree-SHA512: 9322ab1a2f7561b4221fb2bbe9f822c402f845c52a93de14008c1e5bc33e5c6f19ebc647ab6615b7c6be137c76cff6a33c6920818a78813de5632eb88c96a876
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>
We don't need support for transactional memory.
> Disable TM clone registry in libgcc. It is enabled in libgcc by default.
> This option helps to reduce code size for embedded targets which do
> not use transactional memory.
https://gcc.gnu.org/install/configure.html
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
