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
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) {
```
