Move block policy fee estimates from fee_estimates.dat to
fees/block_policy_estimates.dat.
On startup, migrate the legacy file to the new path when only the legacy
file exists. If both files exist, keep the new file and remove the
legacy file.
Rename the block policy estimator args source files to the generic
estimator_args.{cpp,h} names and rename FeeestPath to
BlockPolicyFeeEstPath while the path helper is moved into the shared fee
estimator argument code.
25bed560be test: add forward-compat functional test for txindex (sedited)
703304ed8c doc: add release notes for txindex disk usage and downgrading (Andrew Toth)
8e5320a2d2 tests: cover txindex hash prefix collisions and legacy fallback (Andrew Toth)
b75efa19ba txindex: skip bloom filters and legacy lookups for new databases (Andrew Toth)
004d7c098c txindex: hash key prefixes and pack block positions (Andrew Toth)
5a255970fd refactor: move txindex db constants and legacy key to txindex_key.h (Andrew Toth)
327660134c txindex: pass the full block to DB::WriteTxs (Andrew Toth)
42771e7998 txindex: use a new block locator for downgrade safety (Andrew Toth)
4b08baed72 txindex: return optional tx and block hash from FindTx (Andrew Toth)
Pull request description:
The current txindex uses the full 32-byte txid as keys, which takes up about 66 GB of disk space today on mainnet. Using a 5-byte key prefix instead drops the disk usage to 26 GB - cutting the size to less than half.
Using the full 32-bytes is unnecessary since a 5-byte salted siphash will produce collisions in about 1 in 1.1 trillion. Some collisions will occur, but the penalty is just an extra disk read, deserialization and hash.
The tx position can be appended to the key instead of used as a value, and a LevelDB iterator can seek to the prefix and then scan for the correct tx. This is an almost identical approach to `txospenderindex`.
Also instead of storing the file position of the block, we can store only the sequence of the connected block and offset of the transaction in the block. This can be packed into a 6-byte key suffix using 3-byte representations of the sequence and offset in the block. The block file can be recovered by the CBlockIndex that is already in memory. The sequence is mapped to the block hash in the db, so we can lookup the block hash to find the CBlockIndex during reads.
If a tx is not found with this method, we fallback to looking up the legacy entry. With this method a user with an existing db can opt to erase the `indexes/txindex` folder and reindex, or keep the current index and new entries will be appended with the smaller footprint.
The time to index was faster on my machine with this method, 1h19m vs current 1h50m.
Lookups are roughly the same, around 0.2ms per lookup with `getrawtransaction`.
When testing on mainnet, I got 894,549 2-way collisions, 395 3-way collision, and 1 4-way collision that worst case could cause an extra 3 false positives when reading.
ACKs for top commit:
l0rinc:
diff reACK 25bed560be
sedited:
ACK 25bed560be
ajtowns:
ACK 25bed560be
Tree-SHA512: a25c79ca7e722e2f372b65f5fc11c8b194ad49f2240b4881c7e606306aabbd3604aede3f1c33606b467486affac3a3f503638f513c896935cebbc02709cb60d8
c3945bfd2b doc: use derivehdkey in multisig tutorial (Sjors Provoost)
3662e33669 test: use derivehdkey in M-of-N multisig demo (Sjors Provoost)
d9570f0838 rpc: add derivehdkey (Sjors Provoost)
62da9f9614 wallet: add GetExtKey helper (Sjors Provoost)
aaf1548475 wallet: generalize GetActiveHDPubKeys helper (Sjors Provoost)
3821452c4a refactor: add hardened derivation helper (Sjors Provoost)
0ab61caafd rpc: ParsePathBIP32 helper (Sjors Provoost)
e36c4b76e1 util: reject out-of-range BIP32 keypath indices (Sjors Provoost)
ba78c31a00 fuzz: check ParseHDKeypath/WriteHDKeypath round-trip (Sjors Provoost)
8cce969085 Have ParseHDKeypath handle h derivation marker (Sjors Provoost)
fc53077762 test: move parse_hd_keypath test to bip32_tests (Sjors Provoost)
dab525eb77 key: add DeriveExtKey() helper (Sjors Provoost)
Pull request description:
Adds a `derivehdkey` RPC that returns an xpub, or optionally the xprv, at an arbitrary BIP32 path (with at least one hardened step), derived from a wallet HD key.
The main use case is coordinating a multisig setup, where each participant shares an xpub derived at a hardened path (e.g. `m/87h/0h/0h`) distinct from their default single-signature descriptors. See the (updated) `doc/multisig-tutorial.md` and (updated) functional test to see how that workflow improves.
The first commits are some helpful helpers:
- _key: add DeriveExtKey() helper_ - performs the actual derivation
- _test: move parse_hd_keypath test to bip32_tests_ - from `psbt_wallet_tests`
- _Have ParseHDKeypath handle h derivation marker_
- _util: reject out-of-range BIP32 keypath indices_ - `ParseHDKeypath` would previously map overflowing values without `h` to hardened.
- _fuzz: check ParseHDKeypath/WriteHDKeypath round-trip_
- _rpc: ParsePathBIP32 helper_
- _refactor: add hardened derivation helper_ - `HasHardenedDerivation()`, to enforce the "at least one hardened step" rule
- _wallet: generalize GetActiveHDPubKeys helper_ - extracts code from `gethdkeys` which `derivehdkey` needs
- _wallet: add GetExtKey helper_ - reconstruct an xprv from a wallet xpub (analog of `GetKey()`); behavior-preserving prep, also simplifies `gethdkeys`.
Meat and potatoes:
- _rpc: add derivehdkey_ - the RPC itself, plus the `UnusedKey` filter on `GetHDPubKeys` that drives key selection.
- _test: use derivehdkey in M-of-N multisig demo_ - rewrites the functional multisig test to use the RPC and `<0;1>` syntax.
- _doc: use derivehdkey in multisig tutorial_ - same for the prose tutorial.
ACKs for top commit:
pseudoramdom:
code review ACK c3945bfd2b
achow101:
ACK c3945bfd2b
w0xlt:
That being the case, ACK c3945bfd2b
Tree-SHA512: 661f17c9bfe26017eb14c27ba7af37093387100d3baa25f5d29bba9c1aedc40d19afe1bdfc126a18d018857bb02f1fc84386f10b8f4f4b8e9d6f4b0691d9e302
f280f5eb47 wallet: rpc: deprecate removeprunedfunds (David Gumberg)
e5b7785447 test: wallet: resend: avoid internal behavior via removeprunedfunds (David Gumberg)
Pull request description:
Originally added in https://github.com/bitcoin/bitcoin/pull/7558 as a companion to `importprunedfunds`, this RPC has no known helpful use while being both dangerous and a maintenance burden.
Despite what the name says, it allows the deletion of arbitrary transactions, and `importprunedfunds` does not allow the importing of transactions not belonging to the user, and `listtransactions` does not list transactions not belonging to the wallet, so this RPC can only be used to delete transactions actually belonging to the wallet, and in the unlikely event that transactions not belonging to the wallet are present, they cause no harm except for occupying a few bytes on the users disk.
ACKs for top commit:
achow101:
ACK f280f5eb47
polespinasa:
ACK f280f5eb47
pablomartin4btc:
reACK f280f5eb47
Tree-SHA512: ed9c30c50be514d637999b4c8f3fa9b9b1446a5553e3974703638b45d8f55f1291f5cfb82dd2ead6d0743e424e9e3b1edbd56fc04dae3dcdee4d175e2a1ce061
4ca182ca40 doc: clarify alternate_wtxids is empty when only one witness variant (pablomartin4btc)
fa48b5d28e test: assert listsinceblock "removed" reports current canonical wtxid (pablomartin4btc)
9b96ee1288 wallet, test: add unit test for variant txid validation in CWalletTx deserializer (pablomartin4btc)
9de6543cb5 wallet: post-#35501 cleanup in CWalletTx (pablomartin4btc)
Pull request description:
Follow-up cleanups and clarifications after #35501 was merged.
Commit breakdown:
1. _post-[#35501](https://github.com/bitcoin/bitcoin/pull/35501) cleanup in_ `CWalletTx`
- Rename `arg_state` → `new_state` in `Update()` for consistency
- Simplify `RecomputeCanonical()` using `std::ranges::min_element` with a projection lambda (14 lines → 3 lines)
- Add variant txid validation in the `CWalletTx` deserialise constructor: throws `std::runtime_error` if any variant's txid doesn't match the canonical txid deserialized from the stream
- Move `Init()` to `private` and extend it to clear `m_txs` and reset `m_canonical_wtxid`, so a full re-deserialise via `Unserialize()` starts from a clean state
All [suggested](https://github.com/bitcoin/bitcoin/pull/35501#pullrequestreview-4854519083) by ajtowns.
2. _add unit test for variant txid validation in_ `CWalletTx` _deserializer_
3. _assert_ `listsinceblock` "removed" _reports current canonical wtxid_
Documents that removed entries reflect the wallet's current `CWalletTx` state, not a snapshot of the detached block. A future followup could improve this (requires per-block tracking of which witness variant was included).
[Suggested](https://github.com/bitcoin/bitcoin/pull/35501#discussion_r3632044472) by w0xlt.
4. _clarify_ `alternate_wtxids` _is empty when only one witness variant_
[Suggested](https://github.com/bitcoin/bitcoin/pull/35501#discussion_r3632113003) by polespinasa.
ACKs for top commit:
jeanpablojp:
re-ACK 4ca182ca40
achow101:
ACK 4ca182ca40
polespinasa:
ACK 4ca182ca40
Tree-SHA512: 64eadeb11372d904c79edbfd264c4d8dc1b4fe4ce5e3acc301bfeba9e556efb5dce2c684f0e58c7b74e2c687cc7dd77389970662b3fd629ed034a97bcfdfb71c
When there is only one known witness variant for a transaction,
alternate_wtxids is an empty array, analogous to walletconflicts and
mempoolconflicts.
Suggested-by: polespinasa
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
Add an UnusedKey filter to GetHDPubKeys() so the new RPC can prefer
unused(KEY) descriptors before falling back to active descriptors.
Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>
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
This RPC has no helpful use while being both dangerous and a maintenance
burden.
Despite what the name says, it allows the deletion of arbitrary
transactions, and `importprunedfunds` does not allow the importing of
transactions not belonging to the user, and `listtransactions` does not
list transactions not belonging to the wallet, so this RPC can only be
used to delete transactions actually belonging to the wallet, and in the
unlikely event that transactions not belonging to the wallet are
present, they cause no harm except for occupying a few bytes on the
users disk.
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
