Commit Graph

8396 Commits

Author SHA1 Message Date
GuTS805
21d4e0ba75 rpc, wallet, test: fix invalid JSON in HelpExampleRpc curl examples
Several HelpExampleRpc call sites reused CLI-style argument strings
verbatim (missing commas, bare unquoted words, or single backslashes
that are not valid JSON escapes), producing curl examples that fail
JSON parsing as documented. Also fixes a stray trailing quote in the
restorewallet named-argument examples, a missing comma in the
listunspent example, and a wrong-schema string-instead-of-array
listunspent argument caught in review.

Lines touched are converted to raw string literals (or strprintf with a
raw string template) throughout, for consistency and to avoid manual
quote/backslash escaping.

Adds a regression check to rpc_help.py::dump_help() so this class of
bug can't silently reappear.
2026-08-27 22:34:18 +05:30
merge-script
556988790a Merge bitcoin/bitcoin#35592: http: check rpcallowip immediately after accepting connection
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
2026-08-01 16:43:57 +01:00
merge-script
67efced1fc Merge bitcoin/bitcoin#35838: qa: Enable interface_gui.py on macOS
45f5609f2e qa: Enable `interface_gui.py` on macOS (Hennadii Stepanov)

Pull request description:

  This is a follow-up to bitcoin/bitcoin#35551.

  Addresses https://github.com/bitcoin/bitcoin/pull/35551#discussion_r3619539472.

ACKs for top commit:
  maflcko:
    lgtm ACK 45f5609f2e

Tree-SHA512: 2fd8ebd8529d9d2dc0535181995fb126dfc1033e74ed7680e600fa41bf931f047d82095547538a0c94e1254ecd465d72b4a0ff5189663f25425e1477f845863e
2026-07-30 12:32:26 +01:00
Hennadii Stepanov
45f5609f2e qa: Enable interface_gui.py on macOS
This is a follow-up to bitcoin/bitcoin#35551.
2026-07-30 11:38:01 +01:00
Ava Chow
87bc4c74c4 Merge bitcoin/bitcoin#35787: init, rpc: ignore empty addnode values
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
2026-07-29 13:36:10 -07:00
Ava Chow
67998e15c8 Merge bitcoin/bitcoin#35553: test: Add missing test case for getdata requests from blocks-only peers
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
2026-07-29 13:28:20 -07:00
Ava Chow
146988ef6c Merge bitcoin/bitcoin#35551: test: add interface_gui.py to test bitcoin-qt startup
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
2026-07-29 11:32:46 -07:00
w0xlt
90ce21e21d rpc: reject empty node argument in addnode
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.
2026-07-27 01:40:29 -07:00
w0xlt
69465de447 init: ignore empty addnode values
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.
2026-07-27 00:19:46 -07:00
merge-script
b33a7fcd7b Merge bitcoin/bitcoin#34628: p2p: Replace per-peer transaction rate-limiting with global rate limits
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
2026-07-25 12:15:44 +02:00
Ava Chow
6b059d9dbd Merge bitcoin/bitcoin#32800: rpc: Distinguish between vsize and sigop adjusted mempool vsize
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
2026-07-24 15:09:10 -07:00
Ava Chow
11ebbd9072 Merge bitcoin/bitcoin#28463: p2p: Increase inbound capacity for block-relay only connections
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
2026-07-24 14:30:00 -07:00
brunoerg
69ce0dba2a test: add test that EvictTxPeerIfFull only evicts tx-relaying peers
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2026-07-24 14:16:27 +02:00
Martin Zumsande
3ed7f06418 p2p: trigger possible eviction if we support bloom filters and change a peer to tx relay
Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
2026-07-24 14:16:27 +02:00
Amiti Uttarwar
0bd3d3dfa5 init: make inbound tx relay percentage configurable
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>
2026-07-24 14:16:27 +02:00
Amiti Uttarwar
cc59aee196 test: add functional test for inbound maxconnection limits
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2026-07-24 14:16:27 +02:00
Martin Zumsande
1b76e04736 net: increase inbound capacity for block-relay-only connections
..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>
2026-07-24 14:16:27 +02:00
merge-script
610dd320d1 Merge bitcoin/bitcoin#35783: chainparams: remove my testnet3 seed
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
2026-07-24 09:12:03 +02:00
Ava Chow
526673487c Merge bitcoin/bitcoin#34683: rpc: support a formal description of our JSON-RPC interface
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
2026-07-23 10:57:25 -07:00
Sjors Provoost
7295b8be70 chainparams: remove my testnet3 seed 2026-07-23 12:08:26 +02:00
Ava Chow
7b6f9ba7ba Merge bitcoin/bitcoin#34672: mining: add reason/debug to submitSolution and unify with submitBlock
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
2026-07-22 15:50:08 -07:00
Ava Chow
5311b15727 Merge bitcoin/bitcoin#33014: rpc: Fix internal bug in descriptorprocesspsbt when encountering invalid signatures
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
2026-07-22 14:38:42 -07:00
Hennadii Stepanov
fc4ceda8b6 Merge bitcoin-core/gui#949: Fix -Wsfinae-incomplete warnings when building with GCC 16.x
51d36dfd07 qt: Fix `-Wsfinae-incomplete` warnings when building with GCC 16.x (Hennadii Stepanov)

Pull request description:

  According to the CMake documentation for [`AUTOMOC`](https://cmake.org/cmake/help/latest/prop_tgt/AUTOMOC.html), all `moc` output files that are not included in a source file are aggregated into the CMake-generated `<AUTOGEN_BUILD_DIR>/mocs_compilation.cpp`, which is added to the target's sources.

  Within that single translation unit, `moc`-generated code checks the completeness of a signal or slot parameter type while it is still only forward-declared, and the type is completed later, when a subsequently included `moc_*.cpp` file pulls in the header that defines it. GCC 16.x diagnoses this pattern with the `-Wsfinae-incomplete` warning, which is enabled by default.

  Including the `moc` output files at the end of the corresponding source files excludes them from `mocs_compilation.cpp`, so each one is compiled in a translation unit where the relevant types are complete.

  FWIW, Qt itself uses the same approach throughout its own codebase. Also see https://www.youtube.com/watch?v=Cx_m-qVnEjo.

  ---

  Steps to reproduce on the master branch @ 18c05d9301  on Fedora 44 (GCC 16.1.1):
  ```console
  $ cmake --preset dev-mode
  $ cmake --build build_dev_mode -t bitcoind
  $ cmake --build build_dev_mode -t bitcoin-qt
  [166/172] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/mocs_compilation.cpp.o
  In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_bitcoingui.cpp:9,
                   from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:9:
  /home/hebasto/dev/bitcoin-gui/src/qt/bitcoingui.h:67:7: warning: defining ‘BitcoinGUI’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
     67 | class BitcoinGUI : public QMainWindow
        |       ^~~~~~~~~~
  In file included from /usr/include/qt6/QtCore/qobject.h:19,
                   from /usr/include/qt6/QtWidgets/qwidget.h:10,
                   from /usr/include/qt6/QtWidgets/qdialog.h:9,
                   from /usr/include/qt6/QtWidgets/QDialog:1,
                   from /home/hebasto/dev/bitcoin-gui/src/qt/addressbookpage.h:8,
                   from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_addressbookpage.cpp:9,
                   from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:2:
  /usr/include/qt6/QtCore/qmetatype.h:344:64: note: here.  Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
    344 |         static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
        |                                                                ^~~~~~~~~
  In file included from /home/hebasto/dev/bitcoin-gui/src/qt/paymentserver.h:35,
                   from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_paymentserver.cpp:9,
                   from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:27:
  /home/hebasto/dev/bitcoin-gui/src/qt/sendcoinsrecipient.h:15:7: warning: defining ‘SendCoinsRecipient’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
     15 | class SendCoinsRecipient
        |       ^~~~~~~~~~~~~~~~~~
  /usr/include/qt6/QtCore/qmetatype.h:344:64: note: here.  Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
    344 |         static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
        |                                                                ^~~~~~~~~
  In file included from /home/hebasto/dev/bitcoin-gui/src/qt/psbtoperationsdialog.h:13,
                   from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_psbtoperationsdialog.cpp:9,
                   from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:30:
  /home/hebasto/dev/bitcoin-gui/src/qt/walletmodel.h:48:7: warning: defining ‘WalletModel’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
     48 | class WalletModel : public QObject
        |       ^~~~~~~~~~~
  /usr/include/qt6/QtCore/qmetatype.h:344:64: note: here.  Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
    344 |         static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
        |                                                                ^~~~~~~~~
  In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_qvalidatedlineedit.cpp:9,
                   from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:32:
  /home/hebasto/dev/bitcoin-gui/src/qt/qvalidatedlineedit.h:13:7: warning: defining ‘QValidatedLineEdit’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
     13 | class QValidatedLineEdit : public QLineEdit
        |       ^~~~~~~~~~~~~~~~~~
  /usr/include/qt6/QtCore/qmetatype.h:344:64: note: here.  Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
    344 |         static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
        |                                                                ^~~~~~~~~
  In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_rpcconsole.cpp:9,
                   from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:37:
  /home/hebasto/dev/bitcoin-gui/src/qt/rpcconsole.h:43:7: warning: defining ‘RPCConsole’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
     43 | class RPCConsole: public QWidget
        |       ^~~~~~~~~~
  /usr/include/qt6/QtCore/qmetatype.h:344:64: note: here.  Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
    344 |         static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
        |                                                                ^~~~~~~~~
  In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_sendcoinsentry.cpp:9,
                   from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:39:
  /home/hebasto/dev/bitcoin-gui/src/qt/sendcoinsentry.h:26:7: warning: defining ‘SendCoinsEntry’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
     26 | class SendCoinsEntry : public QWidget
        |       ^~~~~~~~~~~~~~
  /usr/include/qt6/QtCore/qmetatype.h:344:64: note: here.  Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
    344 |         static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
        |                                                                ^~~~~~~~~
  [172/172] Linking CXX executable bin/bitcoin-qt
  ```

ACKs for top commit:
  maflcko:
    review ACK 51d36dfd07 🦅

Tree-SHA512: fe48e4925aaccb833bc065aa7f988f22482ef83d3d555e601d3955109f840b221be971c560668eac79506f567999d4e1b2a464ec3717b5cecb9a3eaaa1dbc01b
2026-07-22 10:11:49 +01:00
merge-script
559d042ba2 Merge bitcoin/bitcoin#35736: bitcoin-util: replace netmagic command with getchainparams command
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
2026-07-22 09:23:44 +01:00
Ava Chow
32eb521002 Merge bitcoin/bitcoin#35215: coins: use SipHash-1-3-UJ for CCoinsMap keys
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
2026-07-21 15:43:03 -07:00
merge-script
d1d85263f8 Merge bitcoin/bitcoin#35681: test: cover disconnect on private broadcast peer with relay=false
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
2026-07-20 22:33:43 +02:00
Hennadii Stepanov
51d36dfd07 qt: Fix -Wsfinae-incomplete warnings when building with GCC 16.x
According to the CMake documentation for `AUTOMOC`, all `moc` output
files that are not included in a source file are aggregated into the
CMake-generated `<AUTOGEN_BUILD_DIR>/mocs_compilation.cpp`, which is
added to the target's sources.

Within that single translation unit, `moc`-generated code checks the
completeness of a signal or slot parameter type while it is still only
forward-declared, and the type is completed later, when a subsequently
included `moc_*.cpp` file pulls in the header that defines it. GCC 16.x
diagnoses this pattern with the `-Wsfinae-incomplete` warning, which is
enabled by default.

Including the `moc` output files at the end of the corresponding source
files excludes them from `mocs_compilation.cpp`, so each one is
compiled in a translation unit where the relevant types are complete.

Additionally:
1. Some of the `BitcoinGUI` class's private members are gated with
   `#ifdef ENABLE_WALLET` to prevent `-Wunused-private-field` warnings
   when building with `-DENABLE_WALLET=OFF`.
2. `test/lint/lint-includes.py` is adjusted to allow new `#include`
   statements.
2026-07-20 12:23:51 +01:00
Lőrinc
af50ba8500 test: add shared SipHash vectors
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>
2026-07-17 20:09:58 -07:00
Anthony Towns
7298281ba8 bitcoin-util: replace netmagic command with getchainparams command
Co-Authored-By: ekzyis <ramdip.singhgill@gmail.com>
2026-07-17 14:01:58 +02:00
willcl-ark
ca9ffb8e12 rpc: add OpenRPC discovery alias
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.
2026-07-17 09:40:25 +01:00
will
6eca11175b lint: remove E731 Ruff ignore
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.
2026-07-16 10:29:58 +01:00
will
b52454538b lint: remove E712 Ruff ignore
Use identity checks for literal false values to preserve RPC semantics.
2026-07-16 10:04:42 +01:00
Ava Chow
70d9ec7f3d Merge bitcoin/bitcoin#34538: net: advertise -externalip addresses
dab7f2c984 test: cover -externalip/onlynet interaction in functional test (will)
657a5aa3f3 test: cover -externalip bypassing -onlynet (will)
8c87e32bd3 net: let -externalip bypass -onlynet (will)
f4af02e827 net: add an add_even_if_unreachable argument to AddLocal (will)

Pull request description:

  `-onlynet` is documented to restrict automatic outbound connections, but it also currently prevents `-externalip` addresses from being advertised when their network is not in the `-onlynet` set. This happens because `AddLocal()` rejects addresses outside `g_reachable_nets`, regardless of whether the address was explicitly configured by the user.

  Previous attempts to fix this (#24835 and #25690) removed the `g_reachable_nets` check from `AddLocal()`.

  This PR instead adds an explicit `add_even_if_unreachable` argument to `AddLocal()`. The argument defaults to `false`, and is set to `true` only when adding addresses from `-externalip`.

  As a result, explicitly configured `-externalip` addresses can still be advertised even when their network is excluded by `-onlynet`, while discovered, mapped, bound, Tor control, and I2P SAM addresses continue to use the existing reachable-network filter.

  This keeps the fix scoped to `-externalip` and addresses the concern raised in #25690:

  > I think it might also be weird for a user to activate -onlynet and keep on advertising their clearnet address to the network

  The branch adds unit coverage for `AddLocal()` and functional coverage in `p2p_addr_selfannouncement.py` for `-onlynet=ipv4 -externalip=<onion>`.

  Fixes: #25336
  Fixes: #25669

ACKs for top commit:
  achow101:
    ACK dab7f2c984
  mzumsande:
    re-ACK dab7f2c984
  w0xlt:
    ACK dab7f2c984

Tree-SHA512: a4ac9334b85da8b6902d3850e21d3a1c9d7dce70bcb79182448c8d5684e24462cd6e440385af7aa4420d9582e4dff9dc9e827ca7a6da0363fff2d3c531784d9b
2026-07-14 15:42:50 -07:00
Ava Chow
7bff765d51 Merge bitcoin/bitcoin#35639: external_signer: validate fingerprint from enumerate response
4c9de7d5b3 external_signer: validate fingerprint from enumerate response (Kyle 🐆)

Pull request description:

  `enumeratesigners` takes the `fingerprint` field from the external signer's `enumerate` output and stores it without checking it. That value is later handed back to the signer command as `--fingerprint <value>` (e.g. in `displayaddress`), so a malformed value propagates unchecked.

  A master key fingerprint is 4 bytes, i.e. 8 hex characters. This adds a check that the reported fingerprint is exactly 8 hex characters and throws a clear error otherwise. A functional test covers empty, wrong-length, and non-hex inputs.

ACKs for top commit:
  Sjors:
    utACK 4c9de7d5b3
  achow101:
    ACK 4c9de7d5b3
  sedited:
    ACK 4c9de7d5b3

Tree-SHA512: 7c3303b24e234e13a4c20c0b93552145b9ccffc29d1bae42ce8a2faf548377f051e52f8ffb3924679065b27d15fc7bf3859e5ae32a2bb185738cc29bc0ade486
2026-07-14 14:15:39 -07:00
merge-script
e3554bf361 Merge bitcoin/bitcoin#35579: wallet: reserve walletrescan before checking wallet is at the tip
9e62e4b1f3 test: slow down rescaning process (Pol Espinasa)
336f5a738b wallet: reserve walletrescan before checking wallet is at the tip (Pol Espinasa)

Pull request description:

  `ImportDescriptors` rpc has a race condition where two imports running in parallel can both succeed or fail one of them.

  The race happens when there are two threads A and B trying to importdescriptors at the same time.

  1. Thread A calls `BlockUntilSyncedToCurrentChain()` (holding `cs_wallet` fast, no contention) and then `reserve()`, acquiring the `WalletRescanReserver`. It proceeds to `ProcessDescriptorImport()`, which holds `cs_wallet` for an extended time (specially on slow machines) while importing descriptors.

  2. B reaches `BlockUntilSyncedToCurrentChain()`, which internally does `WITH_LOCK(cs_wallet, ...)`. Since A holds `cs_wallet`, B blocks here for the entire duration of Thread A's descriptors import.

  3. Then A finishes importing, releases `cs_wallet`, rescans (fast in regtest), and sets `fScanningWallet = false`.

  4. B can now continue in `BlockUntilSyncedToCurrentChain()` acquiring `cs_wallet`, and then calls `reserve()` which succeeds because `fScanningWallet` is already `false`. Both imports succeed.

  I don't think the behavior is problematic at all from a usability PoV, but it can be a bad UX if some imports fails and other's no. It also makes testing difficult as race conditions are not easy to test.

  This PR fixes it by calling `reserver.reserve()` before `cs_wallet` is locked, so multiple threads will be aware of currently imports before being stuck at any point. So only one `importdescriptor` call can be done at the same time.

  I think this should fix https://github.com/bitcoin/bitcoin/issues/35544#issuecomment-4763488259

ACKs for top commit:
  achow101:
    ACK 9e62e4b1f3
  nebula-21:
    ACK 9e62e4b1f3
  w0xlt:
    lgtm reACK 9e62e4b1f3

Tree-SHA512: be0027e1a7b77252ed9fb514c3b3311d6905903d4b0bfc1021a1e1c2bb06872ef599647ff8a4536929240717ae9db62fb75374f629cd3046c7192e2b8b4d7344
2026-07-14 09:45:16 +02:00
Anthony Towns
5cde66341a tests: basic functional test for tx rate limiting 2026-07-12 09:12:27 +10:00
Anthony Towns
df31ee57aa net_processing: add a global delay queue for sending txs
Without the per-peer rate limiting, nodes can act as an amplifier for
transaction spam -- receiving many transactions from one node, but
relaying each of them to over 100 other nodes. Limit the impact of this
by providing a global rate limit.

This is implemented using dual token buckets, one that consumes a
token for every transaction, and one that consumes a token for every
serialized byte. This rate limits both per-tx resource usage (eg INV
messages) and overall relay bandwidth.

Main bucket parameters:
 * Count: 14tx/s rate, 420tx (30s) capacity
 * Size: 12MB/600s rate (4-6 blocks per target block interval), 50MB capacity

The size bucket is expected to be large enough to almost never have an
impact in normal usage, even during transaction storms, and is primarily
intended to mitigate attack-like scenarios.

Outbound connections get a separate pair of buckets, with rates boosted
by a 2.5x multiplier.

This avoids the excessive memory and CPU usage due to the 100x multiplier
from the queues being per-peer.

Note that this also reduces the size of INV messages we send for general
tx relay back to a more reasonable level of under 600 txs in 99.999%
of cases.
2026-07-12 09:12:27 +10:00
rustaceanrob
63c5f9d22c test: Remove mock_process.cpp
The previous binary used a number of `Boost.Test` features:
- `boost::unit_test::disable`
- `BOOST_FAIL`
- `boost::exit_test_failure`

This patch duplicates the previous mock process behavior with no boost features.

With the patch we can:
- simplify the test config
- remove a linted boost include
- remove a file that was not actually a test
2026-07-11 11:09:33 +01:00
Bruno Garcia
1fc9277a1c test: cover disconnect on private broadcast peer with relay=false 2026-07-10 14:53:40 -03:00
Ryan Ofsky
c0e91efdb3 Merge bitcoin/bitcoin#35295: validation: fetch block input prevouts in parallel during ConnectBlock
dc1c17c085 doc: add release notes (Andrew Toth)
0e10937184 fuzz: add coins_view_stacked fuzz harness to test concurrent leveldb reads (Andrew Toth)
ce610a6ff4 fuzz: update harnesses to cover CoinsViewOverlay::StartFetching (Andrew Toth)
760fb22dc3 test: add unit tests for CoinsViewOverlay::StartFetching (Andrew Toth)
d69a3b20de doc: update CoinsViewOverlay docstring to describe parallel fetching (Andrew Toth)
ab2a379237 coins: fetch inputs in parallel (Andrew Toth)
fdf283036a coins: add ready flag to InputToFetch (Andrew Toth)
ede11b8314 validation: collect block inputs in CoinsViewOverlay before ConnectBlock (Andrew Toth)
f82043af50 coins: introduce thread pool in CoinsViewOverlay (Andrew Toth)
5bf1c32008 validation: add -prevoutfetchthreads configuration option (Andrew Toth)

Pull request description:

  This PR is a continuation of https://github.com/bitcoin/bitcoin/pull/31132. All outstanding issues raised there have been resolved, but the volume of stale comments can make that change difficult to review.

  Currently, when connecting a block, each input prevout is looked up one at a time. For every input we first check the in-memory coins cache, and on a miss we make a synchronous round-trip to the chainstate LevelDB to read the coin from disk. Because these lookups happen serially as the block is being validated, the disk read latency stacks up and dominates the time spent in `ConnectBlock` whenever many inputs are not already in the cache.

  This PR moves those disk reads onto a pool of worker threads that run in parallel with block connection. Before entering `ConnectBlock` the block is handed to a `CoinsViewOverlay`, which kicks off the workers to begin fetching all of the block's prevouts from disk and warming the cache. The main validation thread continues to do exactly the same work it does today, hitting the cache for each input in order. The only difference is that by the time it asks, the coin is much more likely to already be there. There are no validation logic or consensus behavior changes. This is purely a parallelization of an existing read pattern.

  The number of fetcher threads is configurable via `-prevoutfetchthreads=<n>`, defaulting to 8 and capped at 16. Setting it to 0 disables input fetching entirely and reverts to the previous serial behavior.

  We have measured large performance gains for IBD and `-reindex-chainstate`, as well as worst-case steady-state block connection at the tip. l0rinc ran many thorough benchmarking passes on the original PR across multiple machines, storage types, dbcache sizes[^1], operating systems[^2], and fetcher thread counts[^3]. Many other contributors also posted their benchmark results in the original PR. IBD speedups range from 1.18× to over 3× faster[^4]. Worst-case block connection time for network-attached storage was over 2× faster[^5]. Flamegraph comparisons before and after this change are available[^6].

  On safety: `ConnectBlock` runs while holding `cs_main`, so nothing else in the node can mutate the chainstate while the fetchers are reading it.

  On LevelDB: [concurrent reads are fully supported](https://github.com/bitcoin/bitcoin/blob/master/src/leveldb/include/leveldb/db.h#L44) and [documented as such](https://github.com/bitcoin/bitcoin/blob/master/src/leveldb/doc/index.md#concurrency). We already rely on this in production today against our other LevelDB-backed databases. The `txindex` DB is read by multiple simultaneous HTTP RPC worker threads via the `getrawtransaction` RPC. The `blockfilterindex` DB is called concurrently from both the P2P `cfilters` / `cfheaders` / `cfcheckpt` message handlers on the `msghand` thread, and from the `getblockfilter` RPC on the HTTP RPC worker threads. We have not yet been issuing concurrent reads against the chainstate DB, but there is no LevelDB-side reason we can't. In fact, the chainstate DB is already being touched by more than one thread on master, because LevelDB schedules its own background compaction work.

  For reviewers:

  The main change is `CoinsViewOverlay` gets 1 new public and 2 new private methods.

  - `StartFetching`: public method called in lieu of `CreateResetGuard` before we enter `ConnectBlock`. It still returns a `ResetGuard` so the view is `Reset` before the block it is working on leaves scope. This kicks off worker threads who each just run `while (ProcessInput()) {}` and then return.
  - `StopFetching`: private method called on `Reset` whenever the guard leaves scope or `Flush`. Stops all threads and clears multi threaded state.
  - `ProcessInput`: private method that fetches a single input prevout. Returns `true` if an input was fetched and `false` otherwise. This is the only method on `CoinsViewOverlay` that is called concurrently by multiple threads. Every other method on the overlay is still called synchronously on the main thread.

  The `CoinsViewOverlay::FetchCoinFromBase` method is also extended to lookup the coins fetched from `ProcessInput` first before falling back to `base->PeekCoin`.

  Mutating methods `Reset` and `Flush` are overridden in `CoinsViewOverlay` to call `StopFetching` first.

  [^1]: https://github.com/bitcoin/bitcoin/pull/31132#pullrequestreview-3515011880
  [^2]: https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3767758819
  [^3]: https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3617721711
  [^4]: https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3678847806
  [^5]: https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-4071032270
  [^6]: https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3617315125

ACKs for top commit:
  l0rinc:
    reACK dc1c17c085
  willcl-ark:
    ACK dc1c17c085
  theStack:
    re-ACK dc1c17c085
  ryanofsky:
    Code review ACK dc1c17c085 with changes to StopFetching and AllInputsConsumed checking behavior since last review.

Tree-SHA512: 89c1c2890f65aac5cd546edc44504956c47b6fada256d3b86ced47e6dd8c72f633a4357753b3b9805b9ba6ed02790822090d70578aba2964baf50d7eb956864c
2026-07-08 20:49:48 -04:00
Ava Chow
f0da26cfc8 Merge bitcoin/bitcoin#34997: p2p: Don't participate in addr relay with feelers
b0735336ee p2p: Don't participate in addr relay with feeler connections (Daniela Brozzoni)

Pull request description:

  Feeler connections are short-lived connection made to check that a node is alive, useful for test-before-evict, and for moving addresses from the new to the tried table.

  We currently send a GETADDR message to feelers, but then disconnect before being able to receive a response. This GETADDR is not useful and can be removed.

  I couldn't find any previous discussion about this, but I found PR #22777, that similarly made sure that we don't ask for tx relay to feelers.

  ---

  I noticed this behavior on my peer-observer instance: I would see the number of sent GETADDR messages increase over time, but the number of ADDR messages with >100 addresses received (which are likely GETADDR responses and not self announcements relays) wouldn't increase as much. I later realized that it was my node opening feeler connections, sending a GETADDR, and closing the connection.

  You can see the same behavior using this command - the node is making feeler connections, sending getaddr to them, closing before receiving the addr response:

  ```
  ~ ₿ tail -f ~/.bitcoin/debug.log | grep -E "(Making feeler connection|Added connection to|sending getaddr|feeler connection completed|Received addr: [0-9]{2,} addresses)"

  2026-04-02T13:25:50Z [net] Making feeler connection to xyz.onion:8333
  2026-04-02T13:26:06Z [net] Added connection to xyz.onion:8333 peer=27
  2026-04-02T13:26:08Z [net] sending getaddr (0 bytes) peer=27
  2026-04-02T13:26:08Z [net] feeler connection completed, disconnecting peer=27, peeraddr=xyz.onion:8333
  ```

  On a node that accepts inbounds connections, this command can be used to see in the logs all the nodes that connected, sent a getaddr, and disconnected before receiving a reply. It is possible that these nodes connected to us as a feeler:
  ```
  ~ ₿ cat .bitcoin/debug.log | awk '

    /received: getaddr/ {
        split($0, a, "peer=")
        got_getaddr[a[2]] = $0
    }

    /sending addr/ {
        split($0, a, "peer=")
        sent_addr[a[2]] = 1
    }

    /socket closed/ {
        split($0, a, "peer=")
        id = a[2]

        if (id in got_getaddr && !(id in sent_addr)) {
            print "possible feeler: " got_getaddr[id]
            print "                 " $0
        }

        delete got_getaddr[id]
        delete sent_addr[id]
    }
  '

  possible feeler: 2026-04-01T21:45:13Z [net] received: getaddr (0 bytes) peer=2311974
                   2026-04-01T21:45:13Z [net] socket closed, disconnecting peer=2311974
  possible feeler: 2026-04-02T00:18:58Z [net] received: getaddr (0 bytes) peer=2426389
                   2026-04-02T00:18:58Z [net] socket closed, disconnecting peer=2426389
  ...
  ```

  Then, you can manually inspect one of them:
  ```
  ~ ₿ cat .bitcoin/debug.log | grep -E "peer=2311974"
  2026-04-01T21:45:13Z [net] Added connection peer=2311974
  2026-04-01T21:45:13Z [net] received: version (102 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] sending version (102 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] send version message: version 70016, blocks=943279, txrelay=0, peer=2311974
  2026-04-01T21:45:13Z [net] sending wtxidrelay (0 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] sending sendaddrv2 (0 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] sending verack (0 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] receive version message: /Satoshi:27.0.0/: version 70016, blocks=943279, us=x.x.x.x:8333, txrelay=0, peer=2311974
  2026-04-01T21:45:13Z [net] received: wtxidrelay (0 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] received: sendaddrv2 (0 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] received: verack (0 bytes) peer=2311974
  2026-04-01T21:45:13Z New inbound v1 peer connected: version: 70016, blocks=943279, peer=2311974
  2026-04-01T21:45:13Z [net] sending sendcmpct (9 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] sending ping (8 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] sending getheaders (1029 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] initial getheaders (943278) to peer=2311974 (startheight:943279)
  2026-04-01T21:45:13Z [net] received: getaddr (0 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] Advertising address x.x.x.x:8333 to peer=2311974
  2026-04-01T21:45:13Z [net] socket closed, disconnecting peer=2311974
  2026-04-01T21:45:13Z [net] Resetting socket for peer=2311974
  2026-04-01T21:45:13Z [net] sending addrv2 (24665 bytes) peer=2311974
  2026-04-01T21:45:13Z [net] Cleared nodestate for peer=2311974
  ```

ACKs for top commit:
  0xB10C:
    ACK b0735336ee
  achow101:
    ACK b0735336ee
  andrewtoth:
    ACK b0735336ee
  stratospher:
    ACK b073533. didn't see any addr message from feelers in my node's last 24 hours/it would disconnect before addr message is received. so consistent with today's behaviour.

Tree-SHA512: 1ac220dfd8361c4687399546a0d968d268e447446053fb8b90ba6b987482cc038e2ad94e670f33181d08d6d0c576882328bb5d0a8b8b1175a5e2ec31ff051833
2026-07-08 11:49:10 -07:00
Matthew Zipkin
d1ed2a6e25 http: check rpcallowip immediately after accepting connection
Instead of sending 403 Forbidden, disconnect as soon as possible.

To facilitate unit testing, this commit includes a refactor
that moves the subnet allow list and relevant methods
into the HTTPServer class instead of file-scope static scope.
2026-07-08 11:29:26 -04:00
merge-script
3f3e644beb Merge bitcoin/bitcoin#35678: private broadcast: define and use new RPC_LIMIT_EXCEEDED error code ( + other follow-ups)
8ac222484c private broadcast: remove no-op [[nodiscard]] (stickies-v)
191bdcba26 test: align test better with described scenario (stickies-v)
7ad311be18 test: use BOOST_CHECK_EQUAL for PrivateBroadcast::AddResult (stickies-v)
82a02a2a22 rpc: define and use new  RPC_LIMIT_EXCEEDED error code (stickies-v)

Pull request description:

  The server isn't running out of memory when the private broadcast transaction queue is full. Add and use a new `-37` (`RPC_LIMIT_EXCEEDED`) code that can be used whenever a resource is bound and currently at capacity.

  Addresses https://github.com/bitcoin/bitcoin/pull/35406#discussion_r3535904571

  Also includes commits to address other outstanding suggestions/nits from #35406:
  - no-op `[[nodiscard]]`: https://github.com/bitcoin/bitcoin/pull/35406#discussion_r3535923165
  - use `BOOST_CHECK_EQUAL`: https://github.com/bitcoin/bitcoin/pull/35406#discussion_r3384040078
  - improve clarity remove-add test case: https://github.com/bitcoin/bitcoin/pull/35406#discussion_r3519358990

ACKs for top commit:
  instagibbs:
    ACK 8ac222484c
  andrewtoth:
    ACK 8ac222484c
  sedited:
    ACK 8ac222484c

Tree-SHA512: c5e0220060770032f3fea54beaabd1d14179a18df3c3e9c25e9f547ad893cea91e2f4b180dd28ad20f09a3cb6b063fad267caab792ee9727f52a2d49f4508523
2026-07-08 09:56:02 +02:00
Ava Chow
e3b026bf56 Merge bitcoin/bitcoin#34020: mining: add getTransactions(ByWitnessID) IPC methods
9784818442 mining: add getTransactionsByWitnessID() IPC method (Sjors Provoost)
d282ae6883 mining: add getTransactionsByTxID() IPC method (Sjors Provoost)
0d5e4d4712 test: restart node after IPC option override test (Sjors Provoost)
f16b3613cd ipc: Serialize null CTransactionRef as empty Data (Sjors Provoost)
0f466e1094 mempool: add lookup by witness hash (Sjors Provoost)

Pull request description:

  For Stratum v2 custom job declaration to be bandwidth efficient, the pool can request[^0] only the transactions that it doesn't know about.

  The spec doesn't specify how this is achieved, but one method is to call the `getrawtransaction` RPC on each transaction id listed in [DeclareMiningJob](https://stratumprotocol.org/specification/06-Job-Declaration-Protocol?query=DeclareMiningJob#644-declareminingjob-client-server) (or a subset if the pool software maintains a cache). Using RPC is inefficient, made worse by the need to make multiple calls. It also doesn't support queuing by witness id (yet, see #34013).

  This PR introduces two new IPC methods:

  - `getTransactionsById()`: takes a list of `Txid`'s
  - `getTransactionsByWitnessID()`: : takes a list of `Wtxid`'s

  Both return a list of serialised transactions. An empty element is returned for transactions that were not found.

  Unlike the RPC counterpart, the IPC methods do not take advantage of `-txindex`. This could be done in a followup. For `Wtxid` that would involve adding a `-witnesstxindex`.

  I thought about having a single (or overloaded) `getTransactions()` that works with both `Txid` and `Wtxid`, but I prefer that clients are intentional about which one they want.

  A unit and functional test cover the new functionality.

  Sv2 probably only needs `getTransactionsByWitnessID()`, but it's easy enough to just add both.

  To rest with Rust use:
  - https://github.com/2140-dev/bitcoin-capnp-types/pull/11

  [^0]: there's two reasons the pool requests these transactions: to approve the template and to broadcast the block if a solution is found (the miner will also broadcast via their template provider). See also https://github.com/stratum-mining/sv2-spec/issues/170

ACKs for top commit:
  achow101:
    ACK 9784818442
  sedited:
    Re-ACK 9784818442
  ViniciusCestarii:
    Re-ACK  9784818442
  ismaelsadeeq:
    Code review ACK 9784818442

Tree-SHA512: 3c6ceb572ab7d8bd090a8f31b5e331304a7a19a3d1f1551c9c2e1ee41339d76f96ca6c41bd634c87fca0a969e7d9bfa6a16c26fb06c0dd2315f6ca1c76a16a31
2026-07-07 14:29:10 -07:00
Musa Haruna
5d25a0c28d rpc: add vsize_adjusted field to getrawtransaction output for mempool transactions
Extend the `getrawtransaction` RPC to include a new field `vsize_adjusted` when the transaction is in the mempool.
The `vsize_adjusted` field provides the mempool's accounting size for the transaction based on its sigop cost,
which can exceed its serialized vsize under `-bytespersigop` policies.

Test coverage is added to verify the correct calculation and exposure of the `vsize_adjusted` field via `mempool_sigoplimit.py`.
2026-07-07 20:12:39 +01:00
Musa Haruna
eaef8d3111 rpc: add vsize_adjusted and vsize_bip141 field to mempool-related RPCs
This commit adds a new `vsize_adjusted` and `vsize_bip141` field to mempool acceptance and submission RPCs,
including `testmempoolaccept` and `submitpackage`,
to report the sigop-adjusted virtual transaction size and virtual transaction size as defined in BIP 141 respectively.
While `vsize` is now marked as deprecated.

RPC help texts are updated to reflect this addition.
Tests in `mempool_accept.py, mempool_accept, p2p_segwit, rpc_packages, mempool_sigoplimit` are extended
to verify the presence and correctness of the new fields.

Co-authored-by: Gloria Zhao <gloriajzhao@gmail.com>
2026-07-07 20:12:24 +01:00
stickies-v
82a02a2a22 rpc: define and use new RPC_LIMIT_EXCEEDED error code
The server isn't running out of memory when the private broadcast
transaction queue is full. Add a new RPC_LIMIT_EXCEEDED code that
can be used whenever a resource is bound and currently at capacity.
2026-07-07 16:24:31 +01:00
merge-script
4498fa5d5b Merge bitcoin/bitcoin#35406: private broadcast: limit outstanding txs to count of 10,000
4e29de719e private broadcast: add release note for limited cap (Gregory Sanders)
cbf8c107c1 Release cs_main between individual private tx re-attempts (Greg Sanders)
5aea3d0373 private broadcast: limit outstanding txs to count of 10,000 (Gregory Sanders)

Pull request description:

  Add a belt-and-suspenders feature, limit the amount of memory and cpu possible when unlucky or simply misconfigured. The worst case limit is roughly 400kB * 10,000 = 4GB, regardless of usage pattern.

  Before this change, sheer volume of broadcasts, mismatches in standardness rules, or simply fee mismatches may result in unbounded growth of memory usage. As the feature may be expanded in the future, explicit bounds helps reasoning going forward.

ACKs for top commit:
  frankomosh:
    tACK 4e29de719e. Ran private_broadcast_tests and p2p_private_broadcast_cap.py. Great to have an explicit bound as the belt-and-suspenders against unbounded queue growth.
  vasild:
    ACK 4e29de719e
  andrewtoth:
    ACK 4e29de719e
  stickies-v:
    ACK 4e29de719e

Tree-SHA512: 18161755f37d07cca185a09e782dbe2fd0025b8befd4f6660e988865cc3a9b705d41769b816161e8142fe6ce31a56e0288bd78efc25135cedfc47fc855011799
2026-07-07 15:10:13 +01:00
will
dab7f2c984 test: cover -externalip/onlynet interaction in functional test
Extend p2p_addr_selfannouncement to restart the node with -onlynet=ipv4
-externalip=<onion> and verify the onion address appears in
localaddresses despite its network being unreachable.
2026-07-07 10:49:38 +01:00
Ava Chow
69fc991791 Merge bitcoin/bitcoin#32606: p2p: Drop unsolicited CMPCTBLOCK from non-HB peer and when blocksonly
55e3a57f22 qa: Avoid UTXO reuse between test functions (Hodlinator)
9c5dd2926a p2p: Ignore CMPCTBLOCK from peer that hasn't sent SENDCMPCT (David Gumberg)
bf9884f4e5 p2p: make blocksonly nodes ignore CMPCTBLOCK messages (David Gumberg)
92cea63c71 test: (Un)solicited invalid cb -> get disconnected. (David Gumberg)
e845e26344 test: p2p: Nodes ignore unsolicited CMPCTBLOCK's (David Gumberg)
8313591715 p2p: Drop unsolicited CMPCTBLOCK from non-HB peer (David Gumberg)
44f377a71f refactor: test: Static assert_highbandwidth_states (David Gumberg)
25457a3272 test: Tighten getblocktxn checks in parallel cb reconstruction test. (David Gumberg)
51dd90fb50 refactor: Merge announce_cmpct_block() defs into one (Hodlinator)

Pull request description:

  Processing unsolicited `CMPCTBLOCK`'s from a peer that has not been marked high bandwidth is not well-specified behavior in BIP-0152, in fact the BIP seems to imply that it is not permitted:

  > "[...] method is not useful for compact blocks because `cmpctblock` blocks can be sent unsolicitedly in high-bandwidth mode"

  See https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki#separate-version-for-segregated-witness

  This PR disables processing of CMPCTBLOCK messages in three cases:
  $1$. When the block is unsolicited and from a non-HB peer.
  $2$. When this node is running in `-blocksonly` mode.
  $3$. When the peer has not advertised `CMPCTBLOCK` support with a `SENDCMPCT` message.

  Not processing unsolicited blocks slightly raises the cost of discovering a peer's mempool via `CMPCTBLOCK` as described in #28272. As pointed out there, getting an HB slot is relatively easy, so this does not prevent an attacker from doing this, it just slightly raises the bar.

  Probably more important is not processing `CMPCTBLOCK` messages as a `-blocksonly` node. A blocksonly node has a lot less surface area for leaking its mempool since it does no transaction relay, and leaking a blocksonly node's mempool is pretty dangerous since it is very likely to be the origin for all of the transactions in its mempool.

ACKs for top commit:
  achow101:
    ACK 55e3a57f22
  w0xlt:
    reACK 55e3a57f22
  hodlinator:
    re-ACK 55e3a57f22
  polespinasa:
    lgtm ACK 55e3a57f22

Tree-SHA512: 118bea55adca01dbd6467ba5ae3adf420d960794a6a2c40dd30fcc7d79aa944e01af0f6dd6bd6ff6d33dc9155171f6f4f497cbd7e8eb6d3c4c89e12740b51c05
2026-07-06 15:52:10 -07:00