75f5851927 doc: add release note for REST cache-control headers (w0xlt)
bbe21ac29f doc: document REST cache-control defaults (w0xlt)
862a179556 http: add no-store to dispatcher-generated error responses (w0xlt)
acf45c44c0 rest: add Cache-Control headers to REST responses (w0xlt)
Pull request description:
This PR adds explicit Cache-Control headers to REST responses.
The policy is:
- Immutable data gets: `Cache-Control: public, immutable, max-age=86400`
- Mutable, node-local, and error responses get: `Cache-Control: no-store`
Important details:
- `/block` and `/block/notxdetails` bin/hex, `/blockpart`, `/blockfilter`, `/spenttxouts`, and `/deploymentinfo/<blockhash>.json` are treated as immutable.
- `/block` and `/block/notxdetails` JSON, all `/tx` formats, `/headers`, `/blockfilterheaders`, `/blockhashbyheight`, `/chaininfo`, `/mempool`, `/getutxos`, and `/deploymentinfo.json` are no-store.
- REST errors and HTTP dispatcher-generated errors are no-store.
- Unmatched `/rest` 404s also return no-store, including paths like `/rest/tx`, `/rest/does-not-exist`, and `/rest?x=1`.
Tests were added in `interface_rest.py` to cover successful responses, behavior across a newly mined block, REST errors, and unmatched REST 404s.
Docs were added to `REST-interface.md`, including guidance for overriding the defaults in a reverse proxy or CDN.
Closes#33809
ACKs for top commit:
stickies-v:
re-ACK 75f5851927
pinheadmz:
ACK 75f5851927
sedited:
ACK 75f5851927
Tree-SHA512: 292ccd06ddfc9272c17fa720ce1ea8bb05462337af6460488f70003d3daf31fcf262e68c264522a911bba65ae2b25fc88a1fd422e5664583daf64070231cb062
db74d3390a doc: clarify test placement guidance (Lőrinc)
Pull request description:
**Problem:** `doc/developer-notes.md` does not explain where test coverage belongs in a commit stack, especially when existing behavior is uncovered or a refactor depends on uncovered behavior.
This has led to review questions about whether tests should record current behavior before a change or be added with the final behavior, for example in [#35251](https://github.com/bitcoin/bitcoin/pull/35251#discussion_r3217842286) and [#31212](https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1854105033).
**Fix:** Add a `General Testing` section under the development guidelines explaining when to use automated tests or a manual testing guide and when behavior-preserving work is easy to validate without new tests.
Add a `Commit Structure for Tests` subsection distinguishing existing coverage, simple uncovered changes, non-trivial changes to uncovered behavior, and non-trivial refactors whose preserved behavior is not covered.
Replace the blanket `CONTRIBUTING.md` rule with a link to the detailed guidance.
ACKs for top commit:
maflcko:
lgtm ACK db74d3390a
pablomartin4btc:
ACK db74d3390a
LarryRuane:
ACK db74d3390a
w0xlt:
ACK db74d3390a
sedited:
ACK db74d3390a
Tree-SHA512: a8f3629b9bd59d20b1bc597d1b43fbb1d3cca9f500a8d79a7b62b417cd6b91c7b92cf6e32946171c76eb54e882a58eac94045753d8bb5899acdb48a7d1ccb2bd
f32685315c doc: Install `pkgconf` to find `capnproto` on NetBSD (Hennadii Stepanov)
5964c7229f doc: Switch `pkg-config` package to modern `pkgconf` on NetBSD (Hennadii Stepanov)
9b85c9814d doc: Drop GCC upgrade instructions for NetBSD (Hennadii Stepanov)
Pull request description:
This PR updates the "NetBSD Build Guide" following the latest release 11.0. See commit messages for more details.
ACKs for top commit:
fanquake:
ACK f32685315c
Tree-SHA512: 1138038715957951d79c838a1f06dfe5d641684901f451a0937a5df16c03c01f76443cc4761b6258cb93dc4ba496fe99129deb45dd9ea3d3a56c9762813a3d76
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
6573196e63 doc: Release note for export watchonly wallet gui action (Ava Chow)
cb51f97f6c gui: Menu action for exporting a watchonly wallet (Ava Chow)
5907a5c7dc gui: Add ExceptionSafeConnect that takes a lambda (Ava Chow)
Pull request description:
Allows a user to export a watchonly version of their wallet to be used in an airgapped setup.
Built on https://github.com/bitcoin/bitcoin/pull/32489
ACKs for top commit:
polespinasa:
lgtm ACK 6573196e63
pablomartin4btc:
ACK 6573196e63
hebasto:
ACK 6573196e63.
Tree-SHA512: 30732ecf2ff40dbbd62a8a9974a907fd60f0da89afacce618fb706a02349135dc06d7dfcc11009caba0e020609ab7e586a0ebbeb7cd65940ee2df229d23f0605
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
f5d7cc66ec doc: Discourage adding AI agents as commit authors (sedited)
Pull request description:
The goal of the AI policy is to ensure that contributors maintain the responsibility of understanding the change they are contributing. Adding AI agents as co-authors undermines this. I believe this philosophy should extend to commit co-authors in general: They should only be added if they themselves are capable of fully understanding the commit.
This contribution was sparked by maflcko's comment here: https://github.com/bitcoin/bitcoin/pull/35551#pullrequestreview-4642682025 .
ACKs for top commit:
l0rinc:
ACK f5d7cc66ec
yancyribbens:
ACK f5d7cc66ec
xyzconstant:
ACK f5d7cc66ec
jonatack:
ACK f5d7cc66ec modulo IANAL, IDK if there are copyright issues with using/crediting work by LLM agents
w0xlt:
ACK f5d7cc66ec
pablomartin4btc:
ACK f5d7cc66ec
theStack:
ACK f5d7cc66ec
Tree-SHA512: 134902fcf4748bf991a6c3df6e41d5edbbc0e57c35d15d9d84fe4d30153549c05546ea5fe22226356d01f6df15f0c7177d9b1af62b9f90982a7788613bdd94a9
349c72ee00 net_processing: Drop unnecessary txid arg from InitiateTxBroadcastToAll (Anthony Towns)
12b0dc33c4 doc: Add release note for -txsendrate etc (Anthony Towns)
5cde66341a tests: basic functional test for tx rate limiting (Anthony Towns)
4842903ac1 rpc: report -txsendrate and bucket info via getnetworkinfo (Anthony Towns)
74a47a5207 init: add -txsendrate configuration parameter (Anthony Towns)
6307bd034b net_processing: Provide a 30bpm heartbeat log while inv backlog is in use (Anthony Towns)
df31ee57aa net_processing: add a global delay queue for sending txs (Anthony Towns)
7927650e56 util/tokenbucket.h: Provide a generic TokenBucket class (Anthony Towns)
749bb447f8 txmempool: Drop CompareMiningScoreWithTopology (Anthony Towns)
e1b7490fbc net_processing: Replace CompareInvMempoolOrder (Anthony Towns)
6cfc65d210 txmempool: Add ExtractBestByMiningScoreWithTopology (Anthony Towns)
026f70e05f net_processing: Remove per-peer rate-limiting (Anthony Towns)
46c8c471dc net_processing: bump last_inv_sequence for bip35 messages explicitly (Anthony Towns)
Pull request description:
Per-peer `m_tx_inventory_to_send` queues have CPU and memory costs that scale with both queue size and peer count. Under high transaction volume, this has previously caused severe issues ([May 2023 disclosure][1]) and still can cause measurable delays ([Feb 2026 Runestone surge][2], with the msghand thread observed hitting 100% CPU and queue memory reaching ~95MB).
This PR replaces the per-peer rate limiting with a global queue using dual token buckets (limiting transaction by both count and serialized size). Transactions that arrive within the bucket capacity still relay nearly immediately, but excess transactions queue in a global backlog and drain as the token buckets refill.
Key parameters:
- Count bucket: 14 tx/s, 420 capacity (30s buffer)
- Size bucket: 20 kB/s (~12 MB/600s), 50 MB capacity
- Outbound peers refill faster by a factor of 2.5
Per-peer queues are retained solely for privacy batching and are always fully emptied, removing the old `INVENTORY_BROADCAST_MAX` cap.
This reduces the memory and CPU burden during transaction spikes when the queuing logic is engaged from O(queue * peers) to O(queue), as the queued transactions no longer need to be retained per-peer or re-sorted per-peer.
Design discussion: https://gist.github.com/ajtowns/d61bea974a07190fa6c6c8eaef3638b9
[1]: https://bitcoincore.org/en/2024/10/08/disclose-large-inv-to-send/
[2]: https://bnoc.xyz/t/increased-b-msghand-thread-utilization-due-to-runestone-transactions-on-2026-02-17/81
ACKs for top commit:
sipa:
Code review ACK 349c72ee00. I haven't tested it myself yet (though switched my well-connected node to it now), but the posted benchmarks and analyses look convincing.
instagibbs:
reACK 349c72ee00
mzumsande:
ACK 349c72ee00
Tree-SHA512: 2196a23308cb7fe36738cf638edf5c5b0e9ba32b11c083609fd8b50291e05bb33484f9921f8beab28d94c58d1adddea4c8ae1182a60a7f53f54be7370e2a0e47
29b124416e doc: add release notes for 32800 (Musa Haruna)
5d25a0c28d rpc: add `vsize_adjusted` field to getrawtransaction output for mempool transactions (Musa Haruna)
eaef8d3111 rpc: add `vsize_adjusted` and `vsize_bip141` field to mempool-related RPCs (Musa Haruna)
Pull request description:
### Motivation and Problem
`CTxMemPoolEntry::GetTxSize()` returns the larger of two values: the BIP 141 virtual size (vsize) and the "sigop-adjusted size." This sigop-adjusted size is used by mempool validation and mining algorithms as a safeguard to prevent overfilling blocks with transactions that approach both the weight and signature operation (sigop) limits in a way that could harm block space efficiency.
In the current implementation, the sigop-adjusted size is reported as the "vsize" in RPCs that provide mempool transaction data, such as `getmempoolentry`, `getrawmempool`, `testmempoolaccept`, and `submitpackage`. However, the documentation for these RPCs typically describes this value simply as the "virtual transaction size as defined in BIP 141," without acknowledging the sigop adjustment. Since the reported size may differ from the pure BIP 141 definition, this confuses people as in this [tweet](https://x.com/mononautical/status/1646166180145577990?s=20), discrepancy can be misleading, as the reported size may differ from the pure BIP 141 definition.
### Proposed Solution
To resolve this, all mempool-related RPCs now return two separate fields:
**vsize_adjusted:** the sigop-adjusted size, i.e. max(BIP 141 vsize, sigop-adjusted size), which reflects the value previously returned under the vsize label and continues to drive mempool acceptance and block template scoring.
**vsize_bip141:** the pure BIP 141 virtual size, strictly `ceil(weight/4)`, matching the consensus definition is now reported here in `vsize_bip141` field. `vsize` field in now marked as DEPRECATED and users are advised to use the new `vsize_bip141` field for pure virtual size instead.
This means that clients that depends on mempool policy size reported vsize will use `vsize_adjusted`, while `vsize` is now purely BIP 141.
Additionally, this PR updates the relevant RPC help text to clearly document the distinction between these two sizes, and adds supporting documentation `doc/policy/feerates-and-vsize.md` to better explain fee rates, virtual size calculations, sigop adjustments, and the mempool policy heuristics.
A new field, vsize_adjusted, has also been added to the getrawtransaction RPC result when input information (transaction is in the mempool) is available. Exposing this value provides users with more precise insight into how the transaction’s sigops impact its effective size for policy and fee estimation.
Note: This picks up work from the closed [#27591](https://github.com/bitcoin/bitcoin/pull/27591)
Fixes [#32775](https://github.com/bitcoin/bitcoin/issues/32775)
ACKs for top commit:
achow101:
ACK 29b124416e
hodlinator:
re-ACK 29b124416e
ismaelsadeeq:
Code review ACK 29b124416e
sedited:
ACK 29b124416e
Tree-SHA512: 9322ab1a2f7561b4221fb2bbe9f822c402f845c52a93de14008c1e5bc33e5c6f19ebc647ab6615b7c6be137c76cff6a33c6920818a78813de5632eb88c96a876
c11508406e doc: Update docs that refer to -maxconnections (Martin Zumsande)
69ce0dba2a test: add test that EvictTxPeerIfFull only evicts tx-relaying peers (brunoerg)
3ed7f06418 p2p: trigger possible eviction if we support bloom filters and change a peer to tx relay (Martin Zumsande)
0bd3d3dfa5 init: make inbound tx relay percentage configurable (Amiti Uttarwar)
cc59aee196 test: add functional test for inbound maxconnection limits (Amiti Uttarwar)
1b76e04736 net: increase inbound capacity for block-relay-only connections (Martin Zumsande)
87bca1c2ad net: add options to AttemptToEvictConnection (Martin Zumsande)
Pull request description:
This is joint work with amitiuttarwar.
See issue #28462 for a broader discussion on increasing the number of block-relay-only connections independent of this particular implementation proposal.
We suggest to increase the number of inbound slots allocated to block-relay-only peers by increasing the default maximum connections from 125 to 200, with 50% of inbound slots accessible for tx-relaying peers.
This is a prerequisite for being able to increase the default number of outgoing block-relay-only peers later, because the current inbound capacity of the network is not sufficient.
In order to account for incoming tx-relaying peers separately from incoming block-relay peers, changes to the inbound eviction logic are necessary.
See the next post in this thread for a more detailed explanation and motivation of the changes.
ACKs for top commit:
instagibbs:
ACK c11508406e
achow101:
ACK c11508406e
dergoegge:
crACK c11508406e
marcofleon:
ACK c11508406e
Tree-SHA512: c71e1481eb235429a6c9d7ce771c7bf825f850b135e904ccfa3505112628fef4188b560d0be0847c968e5ece43c1518590069b7e6e2480790d3ef1ce07d1ac38
a434d66025 cmake, translation: Specify English as target language explicitly (Hennadii Stepanov)
4097d6d968 cmake, translation: Sort messages within contexts alphabetically (Hennadii Stepanov)
312ab8ab0a cmake, translation: Skip source locations in TS files (Hennadii Stepanov)
4f553bd0da cmake, translation: Remove TS to XLIFF conversion (Hennadii Stepanov)
8c30055458 translation: Switch to Qt TS source file (Hennadii Stepanov)
Pull request description:
In Bitcoin Core v22.0, we [switched](https://github.com/bitcoin/bitcoin/pull/21694) from Qt TS to XLIFF translation source file to provide more context, specifically [developer notes](https://doc.qt.io/qt-6/i18n-source-translation.html#add-comments-for-translators), to translators on Transifex. That was very useful for translators back then, even though it required some extra complexity on our side.
Since then, Transifex has enabled support for developer notes in [Qt TS files](https://help.transifex.com/en/articles/6223301-qt-linguist) as well.
Therefore, I believe we should thank XLIFF for its service and retire it.
In addition to switching back to Qt TS, this PR introduces a few tweaks to the `lupdate` command (see the corresponding commit messages).
To summarize, this PR brings the following benefits:
1. Removal of obsolete code from the build system.
2. Minimal diffs during translation updates. For a recent example, see https://github.com/bitcoin-core/gui/pull/931. One can also apply the changes from bitcoin/bitcoin#34301 and run `cmake -B build --fresh -DBUILD_GUI=ON && cmake --build build -t translate` to observe the new minimal diff.
3. More stable string hashes on Transifex. They no longer include string `id`s, which makes this PR an alternative to https://github.com/bitcoin/bitcoin/pull/33270.
As a potential drawback, we are tying ourselves back to Qt's proprietary translation file format.
I've created an experimental resource on Transifex based on this branch: https://app.transifex.com/bitcoin/bitcoin/experimental-do-not-translate. Reviewers can use it to observe Transifex's support for the various features on the following messages:
- \# 11 - Developer Notes
- \# 144 - Plurals
- \# 510 - A disambiguation string (provided as a second argument to the [`tr()`](https://doc.qt.io/qt-6/qobject.html#tr) function) added to the string context.
ACKs for top commit:
l0rinc:
Code review ACK a434d66025
achow101:
ACK a434d66025
sedited:
ACK a434d66025
Tree-SHA512: 2f79af707974acd8c955e01c06b41794ae1702964bd5f6d260dba73f2f14d0b4b6e84f502f8515d84585e00db8db5b644cb6c47f91662a7ba5b6990f2d0ba115
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
The existing `CONTRIBUTING.md` rule requires tests to be updated in the same commit that changes behavior, but it does not explain how to handle existing behavior without coverage or refactors whose preserved behavior is not covered.
Add a `General Testing` section under the development guidelines explaining when to use automated tests or a manual testing guide and when behavior-preserving work is easy to validate without new tests.
The `Commit Structure for Tests` subsection distinguishes existing coverage, simple uncovered changes, non-trivial changes to uncovered behavior, and non-trivial refactors whose preserved behavior is not covered.
Add `TODO` comments next to characterization assertions that record incorrect or temporary behavior, then remove the comments when the behavior-changing commit updates the assertions.
Replace the blanket `CONTRIBUTING.md` rule with a link to the detailed guidance.
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Co-authored-by: optout <13562139+optout21@users.noreply.github.com>
Co-authored-by: ryanofsky <ryan@ryandesktop.com>
Co-authored-by: sedited <seb.kung@gmail.com>
Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>
Co-authored-by: Ava Chow <github@achow101.com>
Co-authored-by: Pablo Martin <pablomartin4btc@gmail.com>
2bab6bc73f refactor: Drop support for FreeBSD < 14 (Hennadii Stepanov)
91b5c8a07c refactor: Remove FreeBSD-specific workaround (Hennadii Stepanov)
56701ff6d5 doc: Clarify supported *BSD releases (Hennadii Stepanov)
Pull request description:
This PR establishes a baseline for the oldest *BSD releases supported by Bitcoin Core. Clarifying these minimum requirements paves the way for dropping compatibility code and workarounds for unsupported versions.
The obsolete FreeBSD-specific workaround and version check have been dropped.
ACKs for top commit:
maflcko:
lgtm ACK 2bab6bc73f
willcl-ark:
ACK 2bab6bc73f
theStack:
lgtm ACK 2bab6bc73f
sedited:
ACK 2bab6bc73f
Tree-SHA512: 6d9ca0ff881a60c33fe3aa18a03726426f07f2896b2f56b12804865acfa910aca7efdc1312eb4055e35aab8423d0c2326b89c1da448e01b4fa213f73dfd2b118
fad5809cb9 doc: Update enum class constant naming style guide (MarcoFalke)
Pull request description:
Lately, it seems there are frequent scripted-diffs and refactors to rename ALL_CAPS enum class constant names to something else, due to third-party macro clashes. E.g.:
* https://github.com/bitcoin/bitcoin/pull/35588
* https://github.com/bitcoin/bitcoin/pull/35487
* https://github.com/bitcoin/bitcoin/pull/34454
* etc... (not listing the intermittent pull request force pushes that lead to early CI failures due to macro clashes)
Try to steer away from ALL_CAPS here by discouraging it in new code.
ACKs for top commit:
kevkevinpal:
ACK [fad5809](fad5809cb9)
hebasto:
ACK fad5809cb9.
pablomartin4btc:
ACK fad5809cb9
stickies-v:
ACK fad5809cb9
yuvicc:
ACK fad5809cb9
musaHaruna:
ACK [fad5809](fad5809cb9)
janb84:
ACK fad5809cb9
Tree-SHA512: f652c0127022a5ea131e956aef0a2d8c98f4c4317519475a38e5f527a5f672b93a8e51743c1a588f38ddc795531168b68ab68f86471b61f25229761eff0f3879
6d0ea4cf5b doc: add release notes (Andrew Toth)
a2b1c86903 txospenderindex: disable bloom filters to optimize disk usage (Andrew Toth)
Pull request description:
LevelDB bloom filters are only consulted on `Get` point reads. This can be verified in https://github.com/bitcoin/bitcoin/blob/master/src/leveldb/table/table.cc#L224-L228. `InternalGet` is the only place that consults the filter, and it is only reached via a `Get` or `Exists` point read. The filters are never consulted for iterator seeks with an iterator created via `NewIterator`.
txospenderindex only reads via iterator seeks, so building them is wasted effort and space.
For a db as large as txospenderindex, this results in measurable performance and disk usage.
On master, a full sync took 4h37m, and the resulting db was 85.0 GiB.
On this branch, a full sync took 3h57m, and the resulting db was 80.9 GiB.
So this is a sync speedup of 39 minutes (1.17x), and a disk space reduction of 4.2 GiB.
ACKs for top commit:
l0rinc:
ACK 6d0ea4cf5b
sedited:
Re-ACK 6d0ea4cf5b
fjahr:
Code review ACK 6d0ea4cf5b
Tree-SHA512: fb88b9f9a16ff31562d388e3fd9fd9590c7864dbe6093cd9430ecbce9cdc3f2a8d3fc612aade743d26ad4c6eca1e5dc9b3f1ca28d75caea1209e5c784895405d
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
2b6e767d96 doc: archive release notes for v31.1 (fanquake)
Pull request description:
v31.1 has been tagged: https://github.com/bitcoin/bitcoin/releases/tag/v31.1/.
ACKs for top commit:
willcl-ark:
ACK 2b6e767d96
Tree-SHA512: 29e30534e56ffc0ea6ad077bdded05792bcc1ce3dd277332208658d1ae2b8aa9ef30289fed72be25370fc62621e71a0d2da563fe58726541d17dfe5d365a2470
31abaa264c doc: add an AI contribution policy (will)
Pull request description:
This policy, adapted from ripgrep, f0cec341ab/AI_POLICY.md who in turn adapted it from uv c5187e200d/AI_POLICY.md, works as a reasonable and pragmatic AI contribution policy at this point in time.
It codifies roughly how the project is currently operating, it's expectations when Ai is being used, and what we don't wish to see.
Link to the document directly from the new PR and issue helptext.
ACKs for top commit:
Sjors:
re-ACK 31abaa264c
achow101:
ACK 31abaa264c
sedited:
Re-ACK 31abaa264c
l0rinc:
ACK 31abaa264c
Tree-SHA512: 667bda2d02717889ee6878438b4e4c7155025ae6933ac748b49f7ca2a04c94515bdd04d522a778828995c3e328780521c1734dd0d8cca4711a702fc9f242756f
68cb7840d2 doc: improve offline-signing-tutorial after 32489 (Pablo Martin)
Pull request description:
General improvements noted in the #32489 review and deferred by the author:
- Remove [a stale NOTE](https://github.com/bitcoin/bitcoin/pull/32489#discussion_r3484961398) referencing `walletcreatefundedpsbt`; the tutorial was updated to use the send RPC instead.
- [Fix](https://github.com/bitcoin/bitcoin/pull/32489#discussion_r3484961536) `listtransactions` example output from `{...}` to `[...]`; the RPC returns a JSON array, not an object.
ACKs for top commit:
polespinasa:
ACK 68cb7840d2
Tree-SHA512: 0615f042a98f68d1a3bd71bf04ad0f66aa88b7011572b58e2b349623da0ac334d5d388caeee16e1bd922650e5aa6a9fa7d9d00da9a70d227dad22a78dd9e6b76
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
cddbad325d doc: Add release notes for 32489 (exportwatchonlywallet RPC) (Pablo Martin)
Pull request description:
This is a follow-up to #32489.
ACKs for top commit:
polespinasa:
ACK cddbad325d
Tree-SHA512: bfc14c1d8576395caea9636fcc57182c7fe027e397b8529d60901f7310e30acfe9157fbc0e11f6ad8c16c96cb96df9d7d70d0965c5e8a06d6fa3e9e7b5a65533
This change establishes a baseline for the oldest *BSD releases
supported by Bitcoin Core. Clarifying these minimum requirements paves
the way for dropping compatibility code and workarounds for unsupported
versions.
