When bitcoin-gui is built against OpenBSD's system Qt packages (which
have GLib support), shutdown emits "GLib-CRITICAL **:
g_main_context_pop_thread_default: assertion 'stack != NULL' failed"
messages on stderr, which the test framework treats as a failure.
Set `QT_NO_GLIB=1` so Qt falls back to its poll-based event dispatcher,
which avoids the GLib thread-default context entirely.
0390338692 test: check MuSig import private key warnings (woltx)
5e62fbf09c wallet: check descriptor private key completeness on import (woltx)
cd8d01bf47 descriptors: require complete MuSig private keys (woltx)
Pull request description:
`importdescriptors` currently checks whether all private keys are present by expanding the descriptor and verifying that every expanded origin pubkey has a private key.
This is wrong for MuSig descriptors because expansion includes the synthetic aggregate pubkey. There is no individual private key for that aggregate pubkey, so importing a fully private MuSig descriptor such as `rawtr(musig(A_priv,B_priv))` incorrectly returns:
```
Not all private keys provided. Some wallet functionality may return unexpected errors
```
This PR fixes the issue by making descriptor private-key completeness account for MuSig participant keys, and by having `importdescriptors` use `Descriptor::HavePrivateKeys()` instead of duplicating its own manual completeness check.
The functional test covers both cases:
- `rawtr(musig(A_priv,B_priv))` imports without warnings.
- `rawtr(musig(A_priv,B_pub))` still warns that not all private keys were provided.
ACKs for top commit:
achow101:
ACK 0390338692
theStack:
Code-review ACK 0390338692
Tree-SHA512: a55fb084c63f725a0991556acdfb822f3a5a669f745a00b9f0bf0996b639986cdf5e2be2e8d3d0a2ee3fe5744355f20b40df576601792ef3db698e606629ad52
6304789a18 Wallet, refactor: Remove orphaned EraseWatchOnly function (vicjuma)
Pull request description:
**Summary**
This is a follow-up PR for #28710 by achow101 where `bool LegacyScriptPubKeyMan::RemoveWatchOnly(const CScript &dest)` which was the only remaining caller for the function `EraseWatchOnly` was removed. This PR is just a cleanup to remove the orphaned function.
ACKs for top commit:
jeanpablojp:
ACK 6304789a18
pablomartin4btc:
ACK 6304789a18
Tree-SHA512: 48546b3fa9155d6f9549ce8e77d9fe34cbfc738e6814ac9cd256b85668d0fa325e51404f962d491c6d4989baefd03815c680578bfcc73741cf9c4e65b141f92e
a7b0b5084a doc: fix dead link in txrequest.h (cyb3ralbert)
Pull request description:
The link is dead. Both the file and the whole `blockchainbib/` section return 404. The host certificate expired on 2026-08-07, so the site looks abandoned rather than moved.
It was a local copy of Miller et al. 2015, *Discovering Bitcoin's Public Topology and Influential Nodes*. The [`blockchainbib` bibliography](https://github.com/kernoelpanic/blockchainbib) that distributed it points to `cs.umd.edu/projects/coinscope/coinscope.pdf`, where the original is still accessible.
No other occurrences in the tree.
ACKs for top commit:
l0rinc:
ACK a7b0b5084a
sedited:
ACK a7b0b5084a
Tree-SHA512: 518b3c703417f544c7aa96d94db4cb92a669c0223eaefecd9847378061273258c45939376e47d616651e78eb090110a30ef9dc786506e1ae5d4ac7cd20cd0dcd
ea59f17220 test: cover v0.14.3 wallet migration (w0xlt)
18b8afd093 test: support v0.14.x in dumb_sync_blocks (w0xlt)
Pull request description:
This PR adds test coverage for migrating legacy Bitcoin Core wallets from v0.14.3 (released in 2017) to the descriptor wallet format. The test validates that users can safely upgrade their wallets while preserving all funds, transaction history, and addresses.
This test was originally developed on top of #32977, as it was requested in reviews.
However, since it also increases test coverage, it can be merged independently.
The test covers two wallet migration scenarios:
* Non-HD Wallet Migration - Tests migration of non-HD wallets (created with `-usehd=0`)
* Single Chain HD Wallet Migration - Tests migration of HD wallets from v0.14.3 (`VERSION_HD_BASE`)
The node v0.14.3 cannot be synced using the normal test framework helpers because it does not have the `syncwithvalidationinterfacequeue` RPC, so the test uses `dumb_sync_blocks` to submit blocks from the ancient node to the modern node before migrating the wallet.
Each scenario uses its own dedicated old/new node pair, which keeps the setup isolated and makes this testing best managed in a separate file rather than in the existing migration test files.
On the Windows cross-built CI job, this test is excluded from the main functional test runner and re-run sequentially in an ASCII-only tmpdir, because the v0.14.3 binary cannot handle non-ASCII characters in the temporary directory path.
ACKs for top commit:
furszy:
utACK ea59f17220
pablomartin4btc:
reACK ea59f17220
Tree-SHA512: 35ef7173e10fe52f20db0d6d1f144c5a1343ff406d294ab7d0e346d79546797f3f145f2893d99bb7b57d71aa14580690ab80eae148ec94b91498eb0938b2d15e
089c883c55 test: Add coverage for listreceivedby* excluding "send" addresses (pablomartin4btc)
873c054805 wallet: Exclude non-owned addresses from listreceivedby* (pablomartin4btc)
Pull request description:
Fixes#16159.
`listreceivedbyaddress`/`listreceivedbylabel` with `include_empty=true` walk the entire address book and return every entry that has no matching `mapTally` record — including addresses with a "send" purpose (foreign addresses that got a label via `setlabel`, the GUI, or `addmultisigaddress`) that this wallet never received funds to and doesn't own.
This excludes those via `IsMine()` rather than the address book's `purpose` field, since `purpose` is set inconsistently across several code paths and `IsMine()` is the same check `mapTally` itself is already built from.
Picks up prior work by kouloumos in #25973 and BrandonOdiwuor in #30972, both closed for
inactivity:
- [#25973](https://github.com/bitcoin/bitcoin/pull/25973) filtered on `purpose == "send"` directly. ryanofsky pointed out purpose "is set pretty haphazardly in code" and [suggested](https://github.com/bitcoin/bitcoin/pull/25973#discussion_r1269477246) `IsMine()` instead.
- [#30972](https://github.com/bitcoin/bitcoin/pull/30972) implemented that, then furszy pointed out `IsMine()` only needs to run for addresses missing from `mapTally`, not every one. rkrux further suggested dropping the redundant re-lock in favor of `EXCLUSIVE_LOCKS_REQUIRED` directly on the lambda — matching the existing pattern in `wallet/interfaces.cpp` — and simplifying the branching.
This PR carries that final approach forward on current master. The regression test is a small, standalone addition rather than reviving the test-file "split into subtests" refactor from the earlier PRs, which achow101 [flagged](https://github.com/bitcoin/bitcoin/pull/30972#issuecomment-3688186614) on #30972 as unrelated stylistic churn.
ACKs for top commit:
polespinasa:
lgtm re-ACK 089c883c55
jeanpablojp:
ACK 089c883c55
achow101:
ACK 089c883c55
Tree-SHA512: d45488c93b9294258faaab5d1891ca5e8c4b8d0d4feb298403c7c3f20d6aa08989d548cddd25ccd47a1ed969e4a309ee68ad1541c6121fed39ed534c78c256e7
fad9ab714b test: Append print_suppressions=0 to LSAN_OPTIONS, and suppress bitcoin-qt (MarcoFalke)
Pull request description:
(see commit msg for rationale and background).
To test, one should be able to use the cmake options such as `-DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' --preset=dev-mode -DBUILD_GUI=ON -DSANITIZERS=address` on e.g. Fedora. Then see that the current suppressions file is insufficient, and also confirm that `print_suppressions=0` is required.
ACKs for top commit:
fanquake:
ACK fad9ab714b
Tree-SHA512: 1830b4aeb072fa18b76522a124a268073675da14255e469a6d86ee5de52cd08d5613d0c3bd8a66465b0c4636345c9e967923cd1fb516906a58b614fe0e700033
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
The print_suppressions=0 is required to avoid a CI failure when the
suppressions were used. E.g:
```
$ LSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/lsan:print_suppressions=1" ./bld-cmake/test/functional/interface_gui.py
2026-08-08T10:53:45.864160Z TestFramework (INFO): PRNG seed is: 8358096631255493262
2026-08-08T10:53:45.914748Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_5zx5343v
2026-08-08T10:53:47.029997Z TestFramework (INFO): Test that bitcoin-gui starts up and can be stopped via RPC
2026-08-08T10:53:47.431761Z TestFramework (ERROR): Unexpected exception:
File "./test/functional/test_framework/test_node.py", line 534, in is_node_stopped
raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))
AssertionError: Unexpected stderr -----------------------------------------------------
Suppressions used:
count bytes template
2 181 bitcoin-qt
----------------------------------------------------- !=
```
The general suppression of the qt executables is required to avoid CI
failures for i386 builds. E.g:
```
test 2026-08-05T08:54:08.370427Z TestFramework (ERROR): Unexpected exception:
Traceback (most recent call last):
File "/ci_container_base/ci/scratch_ ₿🧪_/build-i686-pc-linux-gnu/test/functional/interface_gui.py", line 34, in run_test
self.stop_node(0)
~~~~~~~~~~~~~~^^^
File "/ci_container_base/test/functional/test_framework/test_node.py", line 525, in is_node_stopped
assert return_code in expected_ret_code, self._node_msg(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: [node 0] Node returned unexpected exit code (1) vs ((0,)) when stopping
node0 stderr =================================================================
==73449==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 386 byte(s) in 17 object(s) allocated from:
#0 0x5df28a2d in malloc (/ci_container_base/ci/scratch_ ₿🧪_/build-i686-pc-linux-gnu/bin/bitcoin-qt+0x1d0da2d) (BuildId: 496a5df531df278fe395724ae917f266331b3f81)
#1 0xee03c1d1 (<unknown module>)
Indirect leak of 12 byte(s) in 1 object(s) allocated from:
#0 0x5df28a2d in malloc (/ci_container_base/ci/scratch_ ₿🧪_/build-i686-pc-linux-gnu/bin/bitcoin-qt+0x1d0da2d) (BuildId: 496a5df531df278fe395724ae917f266331b3f81)
#1 0xee03c1d1 (<unknown module>)
SUMMARY: AddressSanitizer: 398 byte(s) leaked in 18 allocation(s).
```
77440814bf fuzz: reset SOCKS5 interrupt between inputs (Hao Xu)
Pull request description:
Reset `g_socks5_interrupt` before each `socks5` fuzz input.
`CThreadInterrupt` remains interrupted until explicitly reset. Previously,
inputs executed after the first input setting the interrupt flag inherited its
state. As corpus inputs are shuffled between all-input coverage runs, the
number of affected inputs and the resulting coverage counts could differ.
Tested with the complete 91-input `socks5` corpus. The all-input deterministic
coverage check passes.
ACKs for top commit:
nervana21:
tACK 77440814bf
maflcko:
lgtm ACK 77440814bf
sedited:
ACK 77440814bf
Tree-SHA512: d1b2b33661f9796628fd7eb1f4ddb212b07110ebbfa7516e305f8aa21bde7898b4bf8fc6f6570df22f8cf6380f1287cb9b6135683ad49b2bdbe83ad9a1af23b9
a3ebf8ab60 rpc: fix mempool entry vsize docs (Musa Haruna)
Pull request description:
Follow-up to [#32800](https://github.com/bitcoin/bitcoin/pull/32800#discussion_r3672286132).
This fixes the RPC help text for `MempoolEntryDescription()`. These docs are used by mempool-entry RPCs such as `getmempoolentry`, verbose `getrawmempool`, `getmempoolancestors`, and `getmempooldescendants`.
The vsize fields in those results describe transactions already in the mempool, so they are always returned and there is no `allowed` field. This removes the incorrect optional markers and the `testmempoolaccept`-specific “only present when allowed is true” wording.
No behavior change.
ACKs for top commit:
sedited:
ACK a3ebf8ab60
Tree-SHA512: 60574d120d8b85944cde00ebb151c0e847c25000d5ea822ee4d624c723626a1c4db187fa8d09f2fe4405cccb58492c8ac7f754f3176f5adf85008dfb4dd96f14
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
222855ed11 doc: mention -DWITH_ZMQ=ON in macOS build guide (cyb3ralbert)
Pull request description:
`doc/build-osx.md` currently says:
> Support for ZMQ notifications requires the following dependency.
The `zeromq` dependency is covered, but the `-DWITH_ZMQ=ON` CMake option is not
mentioned anywhere in that section. `WITH_ZMQ` defaults to `OFF`, so following the
guide as written results in a build with ZMQ disabled, even though the user completed
the ZMQ section. I verified this at the configure step on macOS: with `zeromq`
installed and no flag, CMake reports `ZeroMQ ... OFF`; with `-DWITH_ZMQ=ON` it
reports `ON`.
The same wording was added to the BSD build guides in #35283, but `doc/build-osx.md`
was not included.
Docs-only change. No tests run.
ACKs for top commit:
hebasto:
ACK 222855ed11.
Tree-SHA512: b7e7114e18ef10216780f6b549516c79bff67d7a909c5167f9c48cc8d2ef0b10f1524c7656452308914e5cd86425588f9f2dac07d4eeb2fe1e624f5478453d73
6b6d77cc84 windows: remove deprecated codecvt via UTF-8 narrow APIs (kevkevinpal)
Pull request description:
Since #32380 the Windows process code page is UTF-8, so narrow
APIs accept UTF-8 directly. Drop wstring_convert/codecvt and the
related wide process calls (`_wsystem`, `_wexecvp`, `CreateProcessW`)
in favor of `::system`, `_execvp`, and `CreateProcess`.
This should be fine to remove since Bitcoin Core is now on C++20
ACKs for top commit:
hebasto:
re-ACK 6b6d77cc84, only rebased since my [recent](https://github.com/bitcoin/bitcoin/pull/35704#pullrequestreview-4845891494) review.
hodlinator:
ACK 6b6d77cc84
Tree-SHA512: dd8039e4d2ccf5d04e52d845ceed2b9ce6b2c53ee23680e502eae6b4f07228e42ae602ca5079c42615ec7be61741fd77781e61567adcc6f03e960ce5b55b1d9a
Regression test for #16159: an address labeled via setlabel by a
wallet that doesn't own it is assigned a "send" purpose and must not
appear in listreceivedbyaddress/listreceivedbylabel results, even
with include_empty=true.
Co-authored-by: Andreas Kouloumos <kouloumosa@gmail.com>
listreceivedbyaddress/listreceivedbylabel with include_empty=true
walked the full address book and returned every entry with no
matching mapTally record, including addresses with a "send" purpose
(foreign addresses labeled via setlabel, the GUI, or
addmultisigaddress) that the wallet never received funds to and does
not own.
Filter these out via IsMine() rather than the address book's
"purpose" field, since purpose is set inconsistently across several
code paths and IsMine() is the same check mapTally itself is already
built from.
Fixes#16159.
Co-authored-by: Brandon Odiwuor <brandon.odiwuor@gmail.com>
b9d573e4a9 fees: Return false for incompatible fee estimates (Hao Xu)
Pull request description:
policy_estimator_io deliberately reuses a CBlockPolicyEstimator because
constructing one for every fuzz input severely reduces throughput.
However, Read() returns true for an incompatible old fee estimates file
without replacing the estimator state. The target then calls Write()
with state loaded by a previous input, making coverage depend on corpus
order.
Return false for incompatible files so the target skips Write() when no
state was loaded. This keeps the estimator reuse optimization instead of
resetting the expensive object before every fuzz input.
For the in-tree production caller, incompatible files remain non-fatal
and the estimator still starts from its default state. Read() now
reports failure, so startup emits one additional non-fatal warning.
Node startup and estimator state are unchanged, as is RPC behavior.
ACKs for top commit:
maflcko:
review ACK b9d573e4a9📩
sedited:
ACK b9d573e4a9
Tree-SHA512: d6ec5986122716ad2c6fb5305626aec71b4b416242791547a12f86fb210d768c87fcb9615fd50259908656766bbe085d55ae90e1d72eaee24d438eecfd9c6dd4
87b080fe2b fuzz: reset the reused mempool in process_message(s) (Hao Xu)
d522fd3196 fuzz: prepare deterministic mempool rebuilds (Hao Xu)
b11456386b fuzz: let the test input toggle IBD in the p2p fuzz targets (Hao Xu)
2a29cee684 test: add helper to reset chainman and mempool (Hao Xu)
2a4ef42d34 fuzz: share a single FakeNodeClock in the chainman-resetting fuzz targets (Hao Xu)
Pull request description:
## Problem
`process_message` and `process_messages` keep the node in IBD (`ResetIbd()`) and
mine their coinbases with the default bare-`OP_TRUE` output script. As a result
`net_processing` returns early at the `IsInitialBlockDownload()` check and never
reaches the transaction-handling path; and even if it did, a tx spending a
bare-`OP_TRUE` coinbase is rejected as `NONSTANDARD` by
`ValidateInputsStandardness`. The reused mempool therefore always stays empty and
that path is never exercised.
## Changes
Both targets now get the same treatment:
1. **Toggle IBD from the test input** — a `bool` decides whether to also
`JumpOutOfIbd()`, exercising both the IBD and non-IBD paths. In
`process_message` it is consumed last, so existing corpus entries read `false`
and are unchanged. In `process_messages` the messages run in a loop, so the
bool must be consumed *first* (see the corpus note below).
2. **Use a spendable `P2WSH_OP_TRUE` coinbase** — both anyone-can-spend (an
`OP_TRUE` witness, no signature) and a standard witness output, so a fuzz-built
tx spending a mature coinbase can actually be accepted into the mempool.
3. **Reset the rng before rebuilding (preparation)** — rebuilding the chainman
(and, in the next commit, the mempool) consumes the global PRNG. Reset it with
`MakeRandDeterministicDANGEROUS()` first so the rebuild is deterministic across
iterations. Mirrors the `cmpctblock` harness.
4. **Reset the reused mempool** — now that the mempool can become non-empty,
rebuild it together with the chainman in `ResetChainmanAndMempool()` when the
block index grew or the mempool changed. A dirty mempool is detected by its
sequence number rather than its size, since a tx can be added and removed
within one iteration (leaving the size unchanged).
## Corpus note
~~In `process_messages` the IBD bool is consumed before the message loop (first
integral read), which shifts the `FuzzedDataProvider` layout. Existing
`process_messages` corpus entries can be migrated by appending a single `0x00`
byte at the end (read as `false`, keeping the IBD path); every other consumed
value stays the same. This is a qa-assets change accompanying this PR.~~
This note no longer applies because the IBD toggle is now consumed inside the
message loop. Appending a single `0x00` byte would not reliably target that bool
or preserve the rest of the input layout.
The accompanying `qa-assets` update should migrate or regenerate the affected
`process_messages` corpus entries for the current layout.
ACKs for top commit:
Crypt-iQ:
crACK 87b080fe2b
maflcko:
review ACK 87b080fe2b🏁
frankomosh:
Review ACK 87b080fe2b
Tree-SHA512: e557b2ca3329767a45fe8315c63df9c3191a3a46a17c5e75ea3e4ad0c25e0e500a687fa650297a386b0a2ebb95503d069089ca5ae3d0a34caab98367aeb28683
fae7ba9aba ci: Fix $BASE_ROOT_DIR installation (MarcoFalke)
Pull request description:
In the ci system, `BASE_ROOT_DIR` has a default value that can be changed. This has problems:
* The docs do not mention that changing the value requires re-building the image, as the value is embedded.
* Many places hard-code the default value, which is confusing and brittle.
Fix all issues by adding docs and replacing the hard-coded default values with `$BASE_ROOT_DIR`.
ACKs for top commit:
willcl-ark:
ACK fae7ba9aba
Tree-SHA512: 6bdfc7acf46962f83664b9a4996b65bddad2f3046c19654a83b89c63a7ac8ac3ab3396f03240d42b091f3eda00b9da50bb49a2d19ded37b0de840a20a7c7e1d9
fa2e76d397 bench: Add base_blob compare bench via uint256 (MarcoFalke)
fa588e9e0f refactor: Mark assertion_fail as [[noreturn]] (MarcoFalke)
faec059dfe refactor: Add uint256::operator<=>() (MarcoFalke)
fa6df14c23 refactor: uint256::operator==() = default (MarcoFalke)
Pull request description:
Some refactors with rationale:
* Default the `uint256` base blob equals operator, because this is standard C++20 practise.
* Add the `uint256` base blob `<=>` operator, because this is standard C++20 practise. Also, `transaction_identifier` already offers such an operator. This allows to remove the non-standard `Compare()` function.
* Add a `[[noreturn]]` to the assertion failure helper that does not return. This is standard C++11 practise.
ACKs for top commit:
optout21:
ACK fa2e76d397
Sjors:
ACK fa2e76d397
purpleKarrot:
ACK fa2e76d397
hebasto:
re-ACK fa2e76d397.
w0xlt:
ACK fa2e76d397 as a simplification/refactor, not as a performance optimization.
Tree-SHA512: 2e64e09fabe71d84c05ed943664417dd1bc1cf6fe6bf31006a58dd9180c458f69915ba7770501537678f1ad51cce16018f79bdf6780e072b73b692ea44b60185
264555af3c rpc: avoid descriptor range counter overflow (Lőrinc)
143a13fb2b test: characterize descriptor range endpoint (Lőrinc)
Pull request description:
**Problem:** The authenticated `scantxoutset`, `scanblocks`, `getdescriptoractivity`, `utxoupdatepsbt`, and `descriptorprocesspsbt` RPCs share a descriptor expansion helper that iterates inclusive `int64_t` ranges with an `int` counter.
A ranged descriptor with an explicit `[begin, end]` range ending at `2^31 - 1` expands that valid position, then overflows when advancing the counter to exit the loop.
Trap-enabled builds terminate, while other builds invoke undefined behavior.
**Fix:** Use `int64_t` for loop control so the one-past-the-end value is representable and every position passed to `Descriptor::Expand()` remains within its existing `int` range.
Related: [#26275](https://github.com/bitcoin/bitcoin/pull/26275) fixed the same endpoint overflow in `deriveaddresses`.
ACKs for top commit:
achow101:
ACK 264555af3c
polespinasa:
ACK 264555af3c
sedited:
ACK 264555af3c
Tree-SHA512: 4326182b5897b6f6672e5f7c7296eafdbb6e3b5ed901d61e8fa2cff9b19d372bb8adb88902368dc520ed400e12dd5264ea68677d9ce0feec76fa2ef55fa0d2f4
fac4b06e99 refactor: Use CLIENT_NAME in buildOpenRPCDoc (MarcoFalke)
fa3aadbc32 refactor: Use self.Arg<bool> in getopenrpcinfo (MarcoFalke)
fa1871a528 refactor: Remove stale NOLINTNEXTLINE above GetAddressInfoBaseFields (MarcoFalke)
fa22647914 rpc: Properly make RPCResult::Type::ANY non-test-only (MarcoFalke)
fa1242dcc0 refactor: Use std::visit in ApplyArgFallback (MarcoFalke)
Pull request description:
Commit 6a1a66c180 attempted to properly render RPC results of the type `ANY`.
However, the commit is incomplete.
Fix it, by properly rendering all `ANY` types.
Moreover, a few trivial refactors after https://github.com/bitcoin/bitcoin/pull/34683 are included here.
ACKs for top commit:
sedited:
ACK fac4b06e99
willcl-ark:
ACK fac4b06e99
Tree-SHA512: b77f1956e4feb9afb93d38245491eec190c4538aec7ac57c327fb2efd86c7da6ee8a1406f81fc18fac06bae77a7d38845a537ca0b8f4ae2d94c38eefd8e88dc1
7502b9ddba fuzz: check http_request body matches framing (ameen-alam)
Pull request description:
The http_request target asserted that ReadBody() returns an empty string. That held for the libevent-based http_libevent::HTTPRequest, where the harness only parsed the request line and headers and never populated a body. Commit 9c20859b5f (PR #35182) replaced libevent with http_bitcoin::HTTPRequest, and the target was switched over in e427c227fa; its LoadBody() now decodes Content-Length and chunked bodies per RFC 9112, so any fully-parsed request carrying a body trips the stale assertion (e.g. "POST / HTTP/1.1\r\nContent-Length: 3\r\n\r\nabc").
Replace the emptiness check with a framing-consistency check that mirrors LoadBody()'s own branch logic: a chunked body is bounded by MAX_BODY_SIZE, a Content-Length body is exactly that many bytes, and a request with neither framing header has no body. This strengthens the target instead of dropping the assertion.
**Steps to reproduce (old assertion):**
Build the fuzz binary and pass this input as a file to the `http_request` target:
`POST / HTTP/1.1\r\nContent-Length: 3\r\n\r\nabc`
→ `test/fuzz/http_request.cpp:49: Assertion 'body.empty()' failed`
**Testing the fix:**
Ran the updated target ~16 min under libFuzzer with ASAN/UBSAN
(14.2M execs, no crashes), plus targeted inputs for each branch:
Content-Length body, chunked, `Transfer-Encoding: identity` + Content-Length,
no framing headers, and `Content-Length: 0`. Happy to contribute the repro
input to qa-assets as a follow-up.
ACKs for top commit:
pinheadmz:
ACK 7502b9ddba
marcofleon:
tACK 7502b9ddba
Tree-SHA512: 4f2eb6bdb3a4556866a84fe0f1d0d8cf506e2efd1b1c7493a99f67ca452b31a140034d418c4064142b1a66c3a6c34b97df0e2b12c21ea86cd4019ffc7cff3b27
1278a5970d net_processing: process unique tx INVs only (Gregory Sanders)
Pull request description:
There is no reason we should process conflicting
advertisements for transactions, as they cannot be both accepted into our mempool.
Avoid processing these and doing spurious work.
Should be no change in observable behavior.
ACKs for top commit:
ajtowns:
ACK 1278a5970d
fjahr:
ACK 1278a5970d
l0rinc:
ACK 1278a5970d
Tree-SHA512: c62ceed2cc634c8c99509a8495e5f9bb6d4d8d050942f709a6539ae4dfe1ec628ce7ab0ec1392656809e0d827d1fecf2e8fb9bb4600d13a5bf9a34c9e9e3ad6e
e98ffd4bd8 doc: fix stale bitcoin_en.xlf reference (cyb3ralbert)
Pull request description:
`doc/release-process.md` still instructs users to create the Transifex resource from `src/qt/locale/bitcoin_en.xlf`, even though that file no longer exists.
It was removed in #34808, which switched the Transifex source to the native Qt `.ts` file. That PR updated the other references to `.xlf` in this document, but this one was missed. The last step in the same list already refers to `bitcoin_en.ts`, as does `.tx/config`.
This patch updates the remaining outdated reference.
Docs-only change. No tests run.
ACKs for top commit:
hebasto:
ACK e98ffd4bd8. This was overlooked in https://github.com/bitcoin/bitcoin/pull/34808.
Tree-SHA512: fc6a295aae8a58e72e3324f52849255fe5c49bc53af71733de44489dd8a5dea86e3dfeaaf01cb47c506e717019a390de97a71505babcb1c98626f5c2fd98274f
fa7bc26d12 test: Check that RPCs do not time out, even under load (MarcoFalke)
fa2bd96cc0 test: Map cli CalledProcessError on server error to JSONRPCException (MarcoFalke)
Pull request description:
It turns out there is no test currently to check that the RPC server does not time out under load. With "load" I mean a flood of trivial payloads. That is, the only work needed is JSON encoding and decoding of (let's say) a block of data of 2 MB or so. This may take a few milliseconds, but should never take more than a few seconds.
So add a test for this.
ACKs for top commit:
enirox001:
ACK fa7bc26d12
sedited:
ACK fa7bc26d12
Tree-SHA512: c60646981b7449c757e9fad499e1cd71030376ffb2ae687c8136c6f70accd0a2d76a4cbfc7dd1bc6626ea3a5310a33616a36cd682cc5c4371ec86aa2c8641aeb
81fcecfe45 Revert "ci: Temporarily remove riscv32 config from GHA matrix" (will)
b283e1751c ci: use mirror for riscv submodules (will)
Pull request description:
The https transport is rate-limited to block AI scrapers.
Switch to a live mirror on fish.foo to re-enable the riscv job.
ACKs for top commit:
maflcko:
lgtm ACK 81fcecfe45
sedited:
ACK 81fcecfe45
Tree-SHA512: 1891046d9847b904a3e4be25b7f313fc2413ad1cc0b2c09b2300f81a2f1f691dad816fa5fcfeeae263f8d5f868d229aca5a7ab3f227ce650d359068488bc1e21
sourceware is blocking/rate-limiting http(s) git requests to combat AI
scrapers.
Switch to a live mirror hosted at fish.foo
Use exported GIT_CONFIG_* to avoid setting `git config --global` which
could clobber a user running on bare host, but still propagate to child
git processes spawned by `make`.
