264555af3c rpc: avoid descriptor range counter overflow (Lőrinc)
143a13fb2b test: characterize descriptor range endpoint (Lőrinc)
Pull request description:
**Problem:** The authenticated `scantxoutset`, `scanblocks`, `getdescriptoractivity`, `utxoupdatepsbt`, and `descriptorprocesspsbt` RPCs share a descriptor expansion helper that iterates inclusive `int64_t` ranges with an `int` counter.
A ranged descriptor with an explicit `[begin, end]` range ending at `2^31 - 1` expands that valid position, then overflows when advancing the counter to exit the loop.
Trap-enabled builds terminate, while other builds invoke undefined behavior.
**Fix:** Use `int64_t` for loop control so the one-past-the-end value is representable and every position passed to `Descriptor::Expand()` remains within its existing `int` range.
Related: [#26275](https://github.com/bitcoin/bitcoin/pull/26275) fixed the same endpoint overflow in `deriveaddresses`.
ACKs for top commit:
achow101:
ACK 264555af3c
polespinasa:
ACK 264555af3c
sedited:
ACK 264555af3c
Tree-SHA512: 4326182b5897b6f6672e5f7c7296eafdbb6e3b5ed901d61e8fa2cff9b19d372bb8adb88902368dc520ed400e12dd5264ea68677d9ce0feec76fa2ef55fa0d2f4
1278a5970d net_processing: process unique tx INVs only (Gregory Sanders)
Pull request description:
There is no reason we should process conflicting
advertisements for transactions, as they cannot be both accepted into our mempool.
Avoid processing these and doing spurious work.
Should be no change in observable behavior.
ACKs for top commit:
ajtowns:
ACK 1278a5970d
fjahr:
ACK 1278a5970d
l0rinc:
ACK 1278a5970d
Tree-SHA512: c62ceed2cc634c8c99509a8495e5f9bb6d4d8d050942f709a6539ae4dfe1ec628ce7ab0ec1392656809e0d827d1fecf2e8fb9bb4600d13a5bf9a34c9e9e3ad6e
There is no reason we should process conflicting
advertisements for transactions, as they cannot be both
accepted into our mempool.
Avoid processing these and doing spurious work.
3ac8b806a6 test: test the result order of a multiple import request is correct (Pol Espinasa)
e4732bf018 test: test invalid or missing timestamp throws importdescriptors (Pol Espinasa)
07fb58b9ef test: Test a locked wallet rejects an empty importdescriptors request (Pol Espinasa)
Pull request description:
In addition to #35179 (already merged) this adds more missing test coverage that was detected while rebasing #34861.
The three tests added checks:
- Locked wallet throws because of being locked if giving an empty importdescriptors request.
- Invalid or missing timestamp throws as a top level RPC error and not a per-item error.
- The order of the requests and the response is the same, even if failing or succeeding.
ACKs for top commit:
nebula-21:
ACK 3ac8b806a6
Bicaru20:
re-ACK 3ac8b806a6
brunoerg:
reACK 3ac8b806a6
Tree-SHA512: b6ba9e16bbdbefcab2529f49f9aab0ae8885bd2d381c6eec36ae442dea1aa2361e6fb339ab5bc2c51c3bef6216d8d939db53e57ec577f05fe54c07fc46f8f255
aeca061086 rpc: reject null for optional parameters (Ruslan Kasheparov)
Pull request description:
Treat explicitly passed `null` as missing for optional RPC parameters that are required in certain contexts.
ACKs for top commit:
achow101:
ACK aeca061086
maflcko:
review ACK aeca061086🥚
sedited:
ACK aeca061086
Tree-SHA512: 60f146085fd20e532ba3cbefdb76d430938168621706a20b2b62a34318499fd72a8c934b08f690f9b72d19ed26581094517a0986586f95bc4b23fa8743b24d11
ddddffda3a doc: Add doc/release-notes-35836.md (MarcoFalke)
fa7fe798c6 wallet: Remove meaningless bool fallback in FundTransaction (MarcoFalke)
Pull request description:
This mostly removes a no-op and meaningless bool fallback in the `fundrawtransaction` RPC.
This allows to remove a `skip_type_check`. This makes validating the JSON schema from https://github.com/bitcoin/bitcoin/pull/34683 more consistent.
Adding the type check here is useful, because:
* The fallback was added in af4fe7fd12 (more than a decade ago). Retaining backwards compat with more than 10-year old clients seems purely theoretical. There were other breaking RPC changes on shorter notice in the meantime. If someone really forgot to update this over the last 10 years, I don't see a downside of notifying them.
* The compat only works for positional args, which seems another small reason to drop it.
* The compat is now fully irrelevant and a no-op, given that watch-only wallet is not a concept
anymore after commit 1337c72198.
* Keeping the compat means that openrpc spec users do not get any type checks at all here.
ACKs for top commit:
polespinasa:
ACK ddddffda3a
sedited:
ACK ddddffda3a
Tree-SHA512: aa5113bd74159ae5a3bf189edc48d011761db98beaf701cd144ba94eac9f525bec2b8eeb53e588b2f20dc9965ebabef2c5ce55a732b8cef2874a95f398ecae8f
fa5cbb8909 uint256: Workaround GCC-14 stringop-overread bug in Compare (Ava Chow)
6c9d76d589 doc: release note for alternate_wtxids in gettransaction (Ava Chow)
99bdcb064c test: compat, ensure downgrade preserves tx witness variants (furszy)
ef2afc6a0a test: Test for wallet txs with alternate wtxids (Ava Chow)
2d55c7a74d wallet: Show alternate wtxids in gettransaction (Ava Chow)
0b1af01bd4 wallet: Replace CWalletTx::SetTx with Update (Ava Chow)
56cf27db4d wallet: Store all witness variants of a transaction (furszy)
798ba6d04f wallet: Make CWalletTx::tx private and use CWalletTx::GetTx to access (Ava Chow)
72ebdd6364 wallet: Remove unused CWalletTx CopyFrom and copy constructor (Ava Chow)
19af439bdf wallet: Deserialize directly in CWalletTx's ctor (Ava Chow)
Pull request description:
When the wallet is presented with a transaction that has the same txid as one already known to the wallet, but has a different witness, instead of ignoring the transaction, store it alongside the known tx. This enables the wallet to be aware of all wtxid variants of its transactions. This also allows for the wallet to be able to calculate fees for replacements better as txs with different witnesses may have different feerates.
Specifically, the wallet stores these alternates in `CWalletTx` and extends the existing `tx` record type to essentially have a vector of transactions appended to the record. In `CWalletTx`, the single transaction is replaced with a map of wtxid to transaction so that all witness variants can still be represented by a single `CWalletTx`. For all of the various things that need the tx from a `CWalletTx`, a single witness variant is chosen to be the canonical tx and returned by `GetTx()`. This canonical tx is written into the same place as the previous single tx was written to in the `tx` record so that wallets can be loaded into previous versions.
To choose the canonical transaction, if any of the variants is confirmed, then that is the canonical one. Otherwise, the witness variant with the least weight is chosen.
An additional change I've included is to make `CWalletTx` RAII. This simplifies some of the implementation and enforces the assumption that a `CWalletTx` always has a transaction.
Lastly, `gettransaction` and `listtransaction` have a new field `alternate_wtxids` to inform users of the wtxids of the witness variants for a transaction, and of course, a test.
Closes#11240
ACKs for top commit:
furszy:
ACK fa5cbb8909
ajtowns:
ACK fa5cbb8909
w0xlt:
ACK fa5cbb8909
Tree-SHA512: ee303b395ab7a0843969f9491f876f4472c6301e968d9db87312edf44f7447245e707dd544356371d5f32fe6a619ee6937c24f3b7899f7a8108090b425f22d8e
f4a6d079c4 qa: Support `get_bind_addrs` and `feature_bind_extra` on illumos (Hennadii Stepanov)
5e96a8fd5a doc: Add `lsof` to Test Suite Dependencies on NetBSD (Hennadii Stepanov)
5d01aa4772 qa: Ignore `lsof` warnings on NetBSD (Hennadii Stepanov)
70352fda03 qa: Strip prefix length from NetBSD `ifconfig` output (Hennadii Stepanov)
1c1735567e doc: Add `lsof` to Test Suite Dependencies on FreeBSD (Hennadii Stepanov)
4cb7f39c2c qa: Drop OpenBSD from supported platforms in `get_bind_addrs` function (Hennadii Stepanov)
8a982eea85 qa: Add `skip_if_no_lsof_on_nonlinux` helper and use it where needed (Hennadii Stepanov)
Pull request description:
This PR is a follow-up to #34256. It extends functional test support to illumos-based OSes and fixes several related issues on the *BSDs.
Changes:
- Make `lsof` an optional functional test dependency via a new `skip_if_no_lsof` helper, consistent with other optional test deps.
- Strip the CIDR prefix length from NetBSD `ifconfig` output (no-op on other platforms).
- Suppress spurious `lsof` warnings on NetBSD.
- Drop OpenBSD from the platforms supported by `get_bind_addrs`.
- Document the `lsof` Test Suite Dependency for FreeBSD and NetBSD.
- Add support for `get_bind_addrs` and `feature_bind_extra` on illumos.
CI runs: https://github.com/hebasto/bitcoin-core-nightly/pull/280.
Addresses https://github.com/bitcoin/bitcoin/pull/34256#issuecomment-4361855749.
ACKs for top commit:
l0rinc:
Lightly tested code review ACK f4a6d079c4
sedited:
utACK f4a6d079c4
Tree-SHA512: 24d943d059f5fa3f5626017eff744836177a41724544355f34b3a31fdf287bd1916bc6e903b598c1c55b61da2ff0f931b4455542d9cff6cf399ef7963096dff4
28641fd195 p2p: reject empty getblocktxn requests (furszy)
9871fb726c p2p: reject filtered block inv early when bloom is disabled (furszy)
aaf9412026 refactor: split p2p_getdata.py in sub-cases (furszy)
Pull request description:
Reject requests that make the node read blocks from disk unnecessarily:
* `getblocktxn` is meant to request the txs a peer is missing. When a
peer sends a `getblocktxn` with an empty index vector, it isn't missing
anything, so it shouldn't have sent the message in the first place.
* The peer should not request a filtered block when the node does not
advertise the `NODE_BLOOM` service. Filtered blocks are built from
the bloom filter, which can be loaded only when the `NODE_BLOOM`
service is offered.
Both are disconnected now.
Note: can be split in two PRs if preferred.
ACKs for top commit:
151henry151:
ACK 28641fd195
l0rinc:
lightly tested ACK 28641fd195
mzumsande:
Code Review ACK 28641fd195
winterrdog:
tested ACK 28641fd195
sedited:
ACK 28641fd195
Tree-SHA512: 787ee0741fb797ea0898daab1bf3d7b3a21d91c9d940fc833e893806e95843e9d5a7a79a55ba67ac0f1550c08f2b6cee5ab2b625c082f56885ae795958cc608c
Descriptor ranges may end at `INT32_MAX`, but the expansion loop counts with `int`.
Incrementing after the final index overflows, terminating the node in `-ftrapv` builds and invoking undefined behavior otherwise.
Use `int64_t` so the final increment stays representable.
55d3cd51a4 doc: add release note describing change for forbidden clients (Matthew Zipkin)
d1ed2a6e25 http: check rpcallowip immediately after accepting connection (Matthew Zipkin)
Pull request description:
This is a follow-up to #35182 addressing a review comment from that PR: https://github.com/bitcoin/bitcoin/pull/35182#pullrequestreview-4322490068
This update to HTTPServer checks the IP subnet allowlist as soon as possible (immediately after receiving a connection from a client) before any data is received. This does not entirely protect the server from the "slow loris" attack or [CWE-400](https://cwe.mitre.org/data/definitions/400.html) but does restrict the attack surface to localhost and clients explicitly allowed by the user.
If a client is not allowed by the list, we disconnect as soon as possible. This is a behavior change from master branch (and previous release with libevent) where `403 Forbidden` was returned (after a potentially large amount request data was written to memory by the server).
To facilitate existing unit tests, this commit includes a refactor that moves the subnet allow list and relevant methods into the HTTPServer class instead of static file scope. This is needed because otherwise the allow list would be empty when the unit tests run.
There is still plenty of refactoring to do in order to modernize `HTTPServer` and de-globalize it, but since this specific issue has a resource allocation guard, I wanted to open it quickly on its own.
ACKs for top commit:
janb84:
ACK 55d3cd51a4
winterrdog:
ACK 55d3cd51a4
w0xlt:
ACK 55d3cd51a4
fjahr:
Code review ACK 55d3cd51a4
Tree-SHA512: 545911f2e4d2f97ab8bc854e9e57c39eb896428f8c349d34c8e8025a1f6bfb8cfd436f381e36af8b87592c07df3e16210819f3eef7943e23c6626030e615fdf5
A getblocktxn msg is only needed when at least one tx is
missing from a compact block. If no txs are missing, the
block can be reconstructed without sending the request.
This avoids reading the requested block from disk
unnecessarily and also alerts the peer operator about
their node's buggy behavior.
A peer should not request filtered blocks from a node that
does not advertise NODE_BLOOM. Perform this check before
looking up the block to avoid an unnecessary disk read.
Note: currently, the request is ignored only after the
block has been read from disk, in the bloom filter
existence check.
The fallback was added in commit
af4fe7fd12, more than a decade ago.
However, it is irrelevant, given that watch-only wallet is not a concept
anymore after commit 1337c72198.
So remove the code.
Can be reviewed with the --ignore-all-space option.
90ce21e21d rpc: reject empty node argument in addnode (w0xlt)
69465de447 init: ignore empty addnode values (w0xlt)
Pull request description:
An empty `addnode=` entry currently creates an empty added-node record. The node then repeatedly attempts to connect to the empty destination:
```text
2026-07-23T19:22:33Z [net] trying v2 connection (manual) to , lastseen=0.0hrs
2026-07-23T19:23:33Z [net] trying v2 connection (manual) to , lastseen=0.0hrs
2026-07-23T19:24:34Z [net] trying v2 connection (manual) to , lastseen=0.0hrs
```
With this change, the node ignores empty `-addnode` values when initializing the connection manager. This preserves the existing startup behavior while avoiding useless connection attempts. Non-empty values are unaffected. Values consisting only of whitespace are ignored as well, and every ignored value is logged.
The `addnode` RPC has the same issue, so it now returns `Error: Node address cannot be empty` instead of adding such a record.
Functional tests verify that the node starts with `addnode=`, that no added-node record is created while non-empty values are still added, and that the RPC rejects empty values.
ACKs for top commit:
l0rinc:
lightly tested ACK 90ce21e21d
achow101:
ACK 90ce21e21d
pablomartin4btc:
ACK 90ce21e21d
furszy:
utACK 90ce21e21d
Tree-SHA512: e3074ff8a4e477f12c1f3331bc941854c2c994290afca85d4695a2ade79d0f230d167aaa9a141e619415164a853c54b7225477d0df8e5b96e0f1480223e188e5
278710a88d test: Add missing test case for getdata requests from blocks-only peers (Roqqit)
Pull request description:
ProcessGetData starts by eagerly processing getdata requests. In this loop, a special case checks for peers that have not requested transaction announcements (ie blocksonly) and ignores those requests. This test prevents regressions for that special case, which is currently not covered by existing tests.
ACKs for top commit:
maflcko:
lgtm ACK 278710a88d
achow101:
ACK 278710a88d
sedited:
ACK 278710a88d
nebula-21:
ACK 278710a88d
Tree-SHA512: 2f96efdd4d27e6f754dbdca74c9bf21214f77ee0a05f79ef9d6eb3d8166793346323d71be80bec7fb93bfcb8d764de24f31e5345951591b105557f30ddaabd0e
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
An empty (or whitespace only) node address cannot be resolved, but would
be added to the added nodes list and retried indefinitely, the same way
that an empty -addnode value was before the previous commits.
Reject it for all commands instead. Returning false from AddNode() would
report the misleading "Node already added" error, so check it here.
An empty -addnode currently creates an unresolvable added-node record
that is retried indefinitely. Ignore empty values instead, retaining
startup compatibility while avoiding useless records and connection
attempts.
Values consisting of whitespace only are ignored as well. They can only
be passed on the command line, as the config file parser trims them
away, and are just as unresolvable.
The ignored values are logged, so that the option not taking effect is
not silent.
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
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
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
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
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
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
1fc9277a1c test: cover disconnect on private broadcast peer with relay=false (Bruno Garcia)
Pull request description:
Currently, there is no test case to verify the disconnection of a private broadcast connection when the peer does not support transaction relay. This PR addresses it.
Can be tested with:
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 27f0a63c55..5d9a8f9c06 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3741,7 +3741,7 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string
} else {
LogDebug(BCLog::PRIVBROADCAST, "Disconnecting: does not support transaction relay (connected in vain), %s",
pfrom.LogPeer());
- pfrom.fDisconnect = true;
+ pfrom.fDisconnect = false;
}
return;
}
```
ACKs for top commit:
Herb-ops:
ACK 1fc9277a1c
w0xlt:
ACK 1fc9277a1c
nebula-21:
ACK 1fc9277a1c
Tree-SHA512: 1fd7ad3a5fc690a25276a9654e297858d95607fa9c696a0a6f53223aeb3dde7f80e836d2cbf835a39487f953b04f73b4b8af0c79a2ee8f58c894259f7c9383ff
Lock SipHash-2-4 behavior into shared vectors before refactoring its round and finalization code.
Store inputs as ordered hex byte blocks so `CSipHasher` and the independent Python implementation hash the same byte sequence, with applicable `PresaltedSipHasher` overloads checked against the same vectors.
Add the 64 official SipHash-2-4 vectors alongside block-partition and empty-block cases for the generic path.
Move randomized generic/fixed comparisons to the integer fuzzer.
SipHash-1-3-UJ coverage can add expected outputs for compatible 8- and 32-byte block sequences.
The Python test reads a build-tree copy so functional-test staging behaves consistently when files are symlinked or copied.
Co-authored-by: Pieter Wuille <pieter@wuille.net>
OpenRPC service discovery specifies `rpc.discover` as the discovery
method name.
Expose `rpc.discover` as an alias for `getopenrpcinfo`, so clients that
expect the standard OpenRPC discovery method can retrieve the same
generated document without changing the existing Bitcoin Core RPC.
Replace assigned lambdas with local functions so Ruff can enforce E731.
For platform-specific immutable file cleanup, store the command as data
instead of creating conditional callbacks.
