*** Please remove the following help text before submitting: *** - #1
Open
Zaidalamari wants to merge 4232 commits into
Open
*** Please remove the following help text before submitting: ***#1Zaidalamari wants to merge 4232 commits into
Zaidalamari wants to merge 4232 commits into
Conversation
75f5851 doc: add release note for REST cache-control headers (w0xlt) bbe21ac doc: document REST cache-control defaults (w0xlt) 862a179 http: add no-store to dispatcher-generated error responses (w0xlt) acf45c4 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 75f5851 pinheadmz: ACK 75f5851 sedited: ACK 75f5851 Tree-SHA512: 292ccd06ddfc9272c17fa720ce1ea8bb05462337af6460488f70003d3daf31fcf262e68c264522a911bba65ae2b25fc88a1fd422e5664583daf64070231cb062
…suppress bitcoin-qt fad9ab7 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 fad9ab7 Tree-SHA512: 1830b4aeb072fa18b76522a124a268073675da14255e469a6d86ee5de52cd08d5613d0c3bd8a66465b0c4636345c9e967923cd1fb516906a58b614fe0e700033
Followup to #35704.
Chunked transfer trailers are just headers that are included at the end of the request. We can parse and validate them with code we already use to read headers. In a future commit we will also be able to use one MAX_HEADERS_SIZE limit to cover both sections. Even though we parse and validate, we ignore these data.
The property only has an effect when building for WIN32. Checking for WIN32 before setting the property is redundant.
CMake ignores .rc and .manifest files when not building for WIN32.
CMake ignores .rc files when compiling for non-Windows platform. Checking for WIN32 before adding an .rc file to sources is redundant.
This fixes the build when using recent clang releases and libc++.
…vedby* 089c883 test: Add coverage for listreceivedby* excluding "send" addresses (pablomartin4btc) 873c054 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](#25973) filtered on `purpose == "send"` directly. ryanofsky pointed out purpose "is set pretty haphazardly in code" and [suggested](#25973 (comment)) `IsMine()` instead. - [#30972](#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](#30972 (comment)) on #30972 as unrelated stylistic churn. ACKs for top commit: polespinasa: lgtm re-ACK 089c883 jeanpablojp: ACK 089c883 achow101: ACK 089c883 Tree-SHA512: d45488c93b9294258faaab5d1891ca5e8c4b8d0d4feb298403c7c3f20d6aa08989d548cddd25ccd47a1ed969e4a309ee68ad1541c6121fed39ed534c78c256e7
…-HD and Single Chain) ea59f17 test: cover v0.14.3 wallet migration (w0xlt) 18b8afd 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 ea59f17 pablomartin4btc: reACK ea59f17 Tree-SHA512: 35ef7173e10fe52f20db0d6d1f144c5a1343ff406d294ab7d0e346d79546797f3f145f2893d99bb7b57d71aa14580690ab80eae148ec94b91498eb0938b2d15e
This change updates the vcpkg manifest baseline from the "2025.08.27 Release" to the "2026-07-29 Release", with the following package changes: - boost: 1.88.0 --> 1.91.0 - sqlite3: 3.50.4 --> 3.53.4 - zeromq: 4.3.5#2 --> 4.3.5#3 - qtbase: 6.9.1 --> 6.11.1#1 - qttools: 6.9.1 --> 6.11.1 - libqrencode: 4.1.1#2 --> 4.1.1#3
At the current builtin baseline, the only default feature is `json1`, which enables SQLite's JSON functionality. Bitcoin Core does not use it, and the depends build explicitly omits it with `-DSQLITE_OMIT_JSON`.
- Rename arg_state to new_state in Update() declaration to match implementation - Replace RecomputeCanonical manual loop with std::ranges::min_element - Add variant txid validation in the deserialize constructor - Make Init() private and have it clear all members including m_txs Co-authored-by: Anthony Towns <aj@erisian.com.au>
When a block is detached, listsinceblock "removed" entries reflect the wallet's current CWalletTx rather than a snapshot of the variant that was actually in the detached block. Add assertions to make this behaviour explicit. A future followup could improve listsinceblock to track and report the specific witness variant that was in the disconnected block (requires per-block tracking of which witness variant was included). Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>
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
a7b0b50 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 a7b0b50 sedited: ACK a7b0b50 Tree-SHA512: 518b3c703417f544c7aa96d94db4cb92a669c0223eaefecd9847378061273258c45939376e47d616651e78eb090110a30ef9dc786506e1ae5d4ac7cd20cd0dcd
6304789 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 6304789 pablomartin4btc: ACK 6304789 Tree-SHA512: 48546b3fa9155d6f9549ce8e77d9fe34cbfc738e6814ac9cd256b85668d0fa325e51404f962d491c6d4989baefd03815c680578bfcc73741cf9c4e65b141f92e
…checks on `importdescriptors` 0390338 test: check MuSig import private key warnings (woltx) 5e62fbf wallet: check descriptor private key completeness on import (woltx) cd8d01b 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 0390338 theStack: Code-review ACK 0390338 Tree-SHA512: a55fb084c63f725a0991556acdfb822f3a5a669f745a00b9f0bf0996b639986cdf5e2be2e8d3d0a2ee3fe5744355f20b40df576601792ef3db698e606629ad52
13da2d6758 Merge bitcoin-core/leveldb-subtree#64: refactor: add [[noreturn]] attribute to BackgroundThread* 8c6628bf14 refactor: add [[noreturn]] attribute to BackgroundThread* 8e89537cef Merge bitcoin-core/leveldb-subtree#62: ci: Add LevelDB test workflow 47abe23ff2 ci: run LevelDB tests in GitHub Actions git-subtree-dir: src/leveldb git-subtree-split: 13da2d6758396bd21bf76ca76f514700f82b2a62
e869105 test: Unroll `&&` conditions in macros (rustaceanrob) Pull request description: Picked from #35713. Given that I think this is a strict debugging improvement, I opened as a separate pull: Using `&&` in `BOOST_CHECK` is problematic as failures will not indicate which condition failed. By unrolling these checks, the user knows exactly which expression is the failing case. As an example, here is a line that would be particularly hard to debug if it failed: ``` src/test/net_tests.cpp BOOST_CHECK((*ret)[1] && (*ret)[1]->m_type == "headers" && std::ranges::equal((*ret)[1]->m_recv, MakeByteSpan(msg_data_2))); ``` If any one of these conditions fail, the whole expression fails, with no values printed or indication as to which condition failed. This is also required when using test macros that support value decomposition, which requires `&&` and `||` are `delete`. Examples include `BOOST_TEST`, doctest, Catch2, etc. ref: https://catch2-temp.readthedocs.io/en/latest/assertions.html#other-limitations ref: https://fekir.info/post/decomposing-an-expression/ ACKs for top commit: maflcko: re-ACK e869105 🌽 ismaelsadeeq: reACK e869105 sedited: ACK e869105 Tree-SHA512: 9eb74cecd47ee4fdc3f53beb7d50d5056d543303d023c68b8d47cbe52d37f1156488c8b943faf68dd52c192c43626037bbf08172e7cc24753e0f6070db6e3ab2
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.
99497b3 cmake: Unconditionally add .rc files to sources (Daniel Pfeifer) 654a4cf cmake: Use builtin support for .manifest files (Daniel Pfeifer) 8f69537 cmake: Unconditionally set WIN32_EXECUTABLE target property (Daniel Pfeifer) Pull request description: Remove some redundant logic from the CMake code: * The `WIN32_EXECUTABLE` target property only has an effect when building for `WIN32`. Checking `WIN32` is redundant. * CMake has builtin support for `.rc` and `.manifest` files. Both may be added to sources unconditionally. They only have an effect when building for `WIN32`. ACKs for top commit: 151henry151: re-ACK 99497b3 hebasto: ACK 99497b3. Tree-SHA512: 3d1a6a86feefff58e06d2e692809349f8e5d50e7ded821da775420a6bbfd54baf7dfdba957a0c7e9fc61b87455addfe67a1cc48dbfe4a8e3055f69bb027109c0
Enables making the fields private later.
Move-only change. Also makes ReadRequest() private.
…me it to bitcoin_http http_bitcoin was mostly used during #35182 to distinguish from http_libevent counterpart: - The http_libevent namespace was introduced around the legacy code in 89c54ae. - The http_bitcoin namespace was introduced in 68b5d28 and extended in subsequent commits. - The http_libevent namespace together with code it contained was removed in 8c1eea0. bitcoin_http is a better name as it is Bitcoin Core's implementation of the HTTP protocol, not HTTP protocol's implementation of bitcoin 402 payment required codes or anything like that. The namespace only remains for a few constants and a type which don't have HTTP in their names.
TxDownloadManagerImpl retains a reference to PeerManagerImpl::m_rng, which is non-thread-safe and guarded by g_msgproc_mutex. BlockConnected runs on the validation background thread while holding only m_tx_download_mutex. Reconsidering an orphan with multiple announcers could therefore use m_rng concurrently with message processing. Regression introduced in 9cc7dc5
Since #35730 the HTTP server reserves file descriptors for its listen sockets and for `-rpcmaxconnections` connected clients (16 by default), so `min_required_fds` in init.cpp grew. On select()-based platforms `available_fds` is capped at FD_SETSIZE, which is 256 on NetBSD. The previous value of 94 no longer fits and every node in the test suite started up with a warning, which the framework treats as unexpected stderr and fails on. Recompute the value with the new accounting (256 - 179 = 77) and update the comment to match the current variable names in init.cpp.
On some systems, such as NetBSD, the non-default `-rpcmaxconnections=128` is too high, so bitcoind refuses to start: ``` Error: Not enough file descriptors available. 256 available, 290 required. ``` The test only needs a value above the default of 16. Use 64 and lower `-maxconnections` in that case so the total fits in 256.
…roducible 7e973cc depends: Make tarball creation from local directory reproducible (Hennadii Stepanov) Pull request description: This guarantees `$(package)_sha256_hash` reproducibility regardless of the default behavior of `$(build_TAR)` and fixes [caching](#36006 (comment)) for the `native_libmultiprocess` package. Steps to reproduce the issue using the master branch @ 2a9e35d: ```console $ mkdir a && cd a && git init $ git remote add origin https://github.com/bitcoin/bitcoin.git $ git fetch --depth 1 origin 2a9e35d $ git checkout FETCH_HEAD $ cd depends $ gmake print-native_libmultiprocess_sha256_hash # Hash A. Compare with Hash B. native_libmultiprocess_sha256_hash=7dd817bfc0ee23c408299907aff13fefb0bd3a54ec66dc14ea15b0cc38c3d9ce $ cd ../../ && sleep 2 $ mkdir b && cd b && git init $ git remote add origin https://github.com/bitcoin/bitcoin.git $ git fetch --depth 1 origin 2a9e35d $ git checkout FETCH_HEAD $ cd depends $ gmake print-native_libmultiprocess_sha256_hash # Hash B. Compare with Hash A. native_libmultiprocess_sha256_hash=34d6f79560c0ff7a4f46bd6bfb4693076546b41f071b6dbf879da45ac8384688 ``` ACKs for top commit: fanquake: ACK 7e973cc willcl-ark: ACK 7e973cc Tree-SHA512: f939cd1b2aca04eaa0f8426858bae3657ee9625f915834980caacfbf80843b952451f4d1c29e27179e533de3e10d31688593e4c25ed32e62392bfbaf9e58dd12
…mework b8a8893 qa: Lower `-rpcmaxconnections` in `interface_http.py` test (Hennadii Stepanov) 6f4109b qa: Reduce `-maxconnections` in the functional test framework (Hennadii Stepanov) Pull request description: This PR follows up on #35730 and fixes a [regression](#35730 (comment)) on NetBSD. Since #35730 the HTTP server reserves file descriptors for its listen sockets and for `-rpcmaxconnections` connected clients (16 by default), so `min_required_fds` in `init.cpp` grew. On select()-based platforms `available_fds` is capped at FD_SETSIZE, which is 256 on NetBSD. The previous value of 94 no longer fits and every node in the test suite started up with a warning, which the framework treats as unexpected stderr and fails on. Recompute the value with the new accounting (256 - 179 = 77) and update the comment to match the current variable names in `init.cpp`. ACKs for top commit: achow101: ACK b8a8893 hodlinator: re-ACK b8a8893 winterrdog: re-ACK b8a8893 Tree-SHA512: d6200cc334b98148d71992b1d085ca8f72ba68d330b26d7ba373a0917cca56a444b8d89b0c0827e2a56242893b268e9f581bffc8a631a2ff20cc51f10db3255e
747cff8 rpc: avoid quadratic output lookups (Lőrinc) Pull request description: **Problem:** Transaction-creation RPCs currently take quadratic time to parse outputs. An authenticated RPC client can therefore tie up a worker with a large request. `sendmany` also holds the wallet lock while parsing, delaying other operations on the same wallet. **Fix:** Parse transaction outputs in linear time by reading corresponding keys and values by index instead of looking up each value by key. **Reproducer:** Run `time build/bin/test_bitcoin --run_test=rpc_tests/parse_outputs` before and after the fix: <details> <summary>parse_outputs test in `rpc_tests.cpp`</summary> ```cpp BOOST_AUTO_TEST_CASE(parse_outputs) { constexpr size_t OUTPUT_COUNT{10'000}; UniValue outputs{UniValue::VOBJ}; for (size_t i{0}; i < OUTPUT_COUNT; ++i) { auto destination{EncodeDestination(WitnessV0ScriptHash{CScript{} << i})}; outputs.pushKVEnd(destination, ValueFromAmount(i + 1)); } const auto parsed_outputs{ParseOutputs(outputs)}; BOOST_REQUIRE_EQUAL(parsed_outputs.size(), OUTPUT_COUNT); for (size_t i{OUTPUT_COUNT}; i > 0; --i) { std::pair expected{CTxDestination{WitnessV0ScriptHash{CScript{} << (i - 1)}}, static_cast<CAmount>(i)}; BOOST_CHECK(parsed_outputs[i - 1] == expected); } } ``` </details> E.g. on my M4 Max with `debug` build: ```python Before ████████████████████ 1.80 s After █████▒░░░░░░░░░░░░░░ 0.50 s -72% ``` Related to #35889 ACKs for top commit: achow101: ACK 747cff8 jonatack: ACK 747cff8 jeanpablojp: tACK 747cff8 hodlinator: ACK 747cff8 Tree-SHA512: 154c9f583f6e7f4154882aeb1ae11c40b327d0ef04147e12a0fee749494ae95314cdfc56baad78723f3383f225d752654be0ae9ed7458b1c73f7a6a80922e3ef
… struct 5e0d7a2 refactor: Drastically narrow scope of http_bitcoin namespace and rename it to bitcoin_http (Hodlinator) 8f9fd86 refactor: Make HTTPRemoteClient fields private (Hodlinator) d72f67f refactor: Expose additional HTTPRemoteClient fields through accessors (Hodlinator) 10bbae3 refactor: Expose HTTPRemoteClient fields to tests through methods (Hodlinator) 5b06d90 refactor: Replace HTTPServer::MaybeDispatchRequestsFromClient() with HTTPRemoteClient::TryReadRequest() (Hodlinator) a1183c0 refactor: Extract Send() and Receive() into HTTPRemoteClient from HTTPServer (Hodlinator) 6d9b61d refactor: Extract HTTPRemoteClient::MaybeDisconnect() from HTTPServer::DisconnectClients() (Hodlinator) 6fec8d6 refactor: Make HTTPRequest fields private (Hodlinator) b8cd772 refactor: Make HTTPRequest::GetHeader() return saner optional type (Hodlinator) e5be0dc refactor: Make HTTPResponse a struct since all fields are public (Hodlinator) Pull request description: The new HTTP server implementation in v32 has `HTTPServer` reaching into and modifying fields of `HTTPRemoteClient` and `HTTPRequest`. This PR encapsulates field data of the latter 2 types which enforces invariants and reduces cognitive load[^1]. Exposing data through accessor methods also implies adding lock annotations. Commits: * Makes `HTTPResponse` a struct since it is used that way. (#35182 (comment)) [^2] * `HTTPRequest`: * Saner return type for `GetHeader()` (old type was mirroring the now removed libevent-wrapper and made later commits ugly). * Make fields private. * Simplifies boolean logic in `HTTPServer::DisconnectClients()`. (#35182 (comment)) * Extraction of `HTTPServer` functions into `HTTPRemoteClient`: Refactors `HTTPRemoteClient` to be more self-contained rather than having `HTTPServer` reach into the fields of other objects. (#35182 (comment), #35182 (comment)) * Severely narrows `http_bitcoin` namespace and renames it to `bitcoin_http` (#35182 (comment)) Follow-up to #35182. [^1]: Core Guidelines: C.9: Minimize exposure of members - https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c9-minimize-exposure-of-members [^2]: Core Guidelines: C.2: Use class if the class has an invariant; use struct if the data members can vary independently - https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c2-use-class-if-the-class-has-an-invariant-use-struct-if-the-data-members-can-vary-independently ACKs for top commit: achow101: ACK 5e0d7a2 janb84: ACK 5e0d7a2 winterrdog: tACK 5e0d7a2 Tree-SHA512: e1c5aa067538e31247ca74923e451038c90750ccc941ae16711dd976c8cd750bd1afaee6e4378aeee91440f7727955d9bfb32aa25a0a745613d0d771a674ebc8
Just to document the current behavior, fixed in the next commit.
80eaa6c bugfix: give TxDownloadManager its own RNG (Greg Sanders) Pull request description: TxDownloadManagerImpl retains a reference to PeerManagerImpl::m_rng, which is non-thread-safe and guarded by g_msgproc_mutex. BlockConnected runs on the validation background thread while holding only m_tx_download_mutex. Reconsidering an orphan with multiple announcers could therefore use m_rng concurrently with message processing. Regression introduced in #35986 Added a regression test on second commit, can remove it from the PR if deemed superfluous. This is a Project Loupe find. ACKs for top commit: maflcko: review ACK 80eaa6c 🐓 hodlinator: ACK 80eaa6c sedited: ACK 80eaa6c Tree-SHA512: 2dbc4a9298bfa1375dc364ead4b1ec74c2ebe54fb7c311180fa06fc32240406be2979a2dd6ae0e7a23b099ddcd63f5c76c126d84a1405fc7eacd337eb009dd88
The following is failing in CI, when a block has `m_height` of 64 bit max: ``` SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /home/runner/work/_temp/src/policy/fees/mempool_estimator.cpp:210:62 MS: 0 ; base unit: 0000000000000000000000000000000000000000 0x1,0x0,0x0,0x0,0x3,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x1,0x0,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x0,0x0,0x26,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x2f,0x0,0x3,0x2,0x2,0x2,0x2,0x2,0x2,0x2,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x7a,0x3f,0x3f,0x0,0x0,0x2f,0x0,0x3,0x2,0x2,0x2,0x2,0x2,0x2,0x2,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x7a,0x3f,0x3f,0x3f,0xff,0xff,0xff,0xff,0xff,0x18,0x0,0x0,0x85,0x3f,0xff,0xff,0xff,0xff,0xff,0x18,0x0,0x0,0x85,0xd6,0x1,0x0,0x86,0x0,0x0,0x0,0x2a,0x0,0xff,0xff,0xff, \001\000\000\000\003\377\377\377\377\377\377\377\377\001\000\377\377\377\377\377\377\377\377\000\000&\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000/\000\003\002\002\002\002\002\002\002\000\000\000\000\000\000\000z??\000\000/\000\003\002\002\002\002\002\002\002\000\000\000\000\000\000\000z???\377\377\377\377\377\030\000\000\205?\377\377\377\377\377\030\000\000\205\326\001\000\206\000\000\000*\000\377\377\377 artifact_prefix='./'; Test unit written to ./crash-b4333d1fe3993fe8610b86654e385682c23050b9 Base64: AQAAAAP//////////wEA//////////8AACYAAAAAAAAAAAAAAAAAAAAAAAAvAAMCAgICAgICAAAAAAAAAHo/PwAALwADAgICAgICAgAAAAAAAAB6Pz8///////8YAACFP///////GAAAhdYBAIYAAAAqAP///w==⚠️ Failure generated from target with exit code 1: ['/home/runner/work/_temp/build_ ₿🧪_/bin/fuzz', '-runs=1', PosixPath('/home/runner/work/_temp/ci/scratch_ ₿🧪_/qa-assets/fuzz_corpora/policy_estimator_io')] Check if using libFuzzer ... True Command '['docker', 'exec', '--env', 'DANGER_RUN_CI_ON_HOST=1', '8100bf684275e706787e07f8ab94431926ba5184562c52bce95c932210f6f38f', '/home/runner/work/_temp/ci/test/03_test_script.sh']' returned non-zero exit status 1. ```
fa72de7 util: Set Univalue to null after read failure (MarcoFalke) fa77865 test: Add UniValue failed read test (MarcoFalke) Pull request description: Currently, `UniValue::read()` may leave the value in a dirty/corrupt state after a read failure. This is perfectly fine, because all production code-paths check the read return value and exit early. However, it seems nicer and safer to discard the dirty and corrupt state. So do that here. This refactor doesn't change any production behavior. However, it fixes a fuzz failure in the `rpc` target, which was recently reworked in commit fa895bb. Later, adding new fuzz inputs (e.g. `fuzz_corpora/rpc/fa1b0eeaa948a091f022c1ff2d0002a3fa6a631f `) and commit 747cff8 made it hit this invalid UniValue code path. ACKs for top commit: rustaceanrob: ACK fa72de7 hodlinator: re-ACK fa72de7 jeanpablojp: tACK fa72de7 Sjors: ACK fa72de7 l0rinc: code review ACK fa72de7 Tree-SHA512: 6c0597a5ab558dc7d22e1742e89078e07a59a87185228114db4381c3381e19b668b95e42cb7ff80288fa56fb15ea1e0e181f59ba3ceaea8a2a3bff12899c7821
576a0eb fix: UB sanitizer in mempool estimator logging (rustaceanrob) Pull request description: The following is failing in CI, when a block has `m_height` of 64 bit max: ``` SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /home/runner/work/_temp/src/policy/fees/mempool_estimator.cpp:210:62 MS: 0 ; base unit: 0000000000000000000000000000000000000000 0x1,0x0,0x0,0x0,0x3,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x1,0x0,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x0,0x0,0x26,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x2f,0x0,0x3,0x2,0x2,0x2,0x2,0x2,0x2,0x2,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x7a,0x3f,0x3f,0x0,0x0,0x2f,0x0,0x3,0x2,0x2,0x2,0x2,0x2,0x2,0x2,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x7a,0x3f,0x3f,0x3f,0xff,0xff,0xff,0xff,0xff,0x18,0x0,0x0,0x85,0x3f,0xff,0xff,0xff,0xff,0xff,0x18,0x0,0x0,0x85,0xd6,0x1,0x0,0x86,0x0,0x0,0x0,0x2a,0x0,0xff,0xff,0xff, \001\000\000\000\003\377\377\377\377\377\377\377\377\001\000\377\377\377\377\377\377\377\377\000\000&\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000/\000\003\002\002\002\002\002\002\002\000\000\000\000\000\000\000z??\000\000/\000\003\002\002\002\002\002\002\002\000\000\000\000\000\000\000z???\377\377\377\377\377\030\000\000\205?\377\377\377\377\377\030\000\000\205\326\001\000\206\000\000\000*\000\377\377\377 artifact_prefix='./'; Test unit written to ./crash-b4333d1fe3993fe8610b86654e385682c23050b9 Base64: AQAAAAP//////////wEA//////////8AACYAAAAAAAAAAAAAAAAAAAAAAAAvAAMCAgICAgICAAAAAAAAAHo/PwAALwADAgICAgICAgAAAAAAAAB6Pz8///////8YAACFP///////GAAAhdYBAIYAAAAqAP///w==⚠️ Failure generated from target with exit code 1: ['/home/runner/work/_temp/build_ ₿🧪_/bin/fuzz', '-runs=1', PosixPath('/home/runner/work/_temp/ci/scratch_ ₿🧪_/qa-assets/fuzz_corpora/policy_estimator_io')] Check if using libFuzzer ... True Command '['docker', 'exec', '--env', 'DANGER_RUN_CI_ON_HOST=1', '8100bf684275e706787e07f8ab94431926ba5184562c52bce95c932210f6f38f', '/home/runner/work/_temp/ci/test/03_test_script.sh']' returned non-zero exit status 1. ``` ACKs for top commit: maflcko: lgtm ACK 576a0eb marcofleon: ACK 576a0eb jeanpablojp: tACK 576a0eb Tree-SHA512: a7533e68a95b2f0200abdcf08ae72a7f3654db03cffe642ed39b0a5aa48d932a8b07ee4e4477b591a488b5358dd19fe85a03015642b663fd41a771b3090bc8a3
Add a method that clears and resets CuckooCache, intended for use in tests only. Without this, fuzz tests may reuse the cache across iterations, resulting in instability.
Co-authored-by: marcofleon <marleo23@proton.me>
2777300 fuzz: Implement connect_block harness (Robin David) 40add91 test: Add reset to CuckooCache (Eugene Siegel) Pull request description: Adds a fuzz target that directly calls `ConnectBlock` with `fJustCheck` set to true, so it hits block/transaction validation without writing undo data or updating the chainstate. This PR is essentially #34651 with some minor tweaks and style cleanups. Additional validation harnesses (e.g. #34895) could build on this test's setup. ACKs for top commit: Crypt-iQ: ACK 2777300 nervana21: tACK 2777300 Tree-SHA512: e2dc74154a6e29e0f3eaec9caeeec53d64bcc96adb0d1739281da97712dd931c3937eaf71f977bfc9c9330f26e35b3633f72788143b76e24e17c37b0a4258ba4
…ment size limit 558e26e test: cover OP_SUCCESSx bypassing the initial stack element size limit (ViniciusCestarii) Pull request description: BIP-342 specifies that the initial stack resource checks happen after OP_SUCCESSx processing, and explicitly notes the checks "can be bypassed using OP_SUCCESSx". Core implements this correctly, but there are currently no tests covering this behavior. This means a consensus-breaking change to the ordering could pass the test suite undetected. Verified this on local commit ViniciusCestarii@68d24d7, which mutates to incorrectly implement the order and CI still turns green. Add a new test at feature_taproot.py to cover OP_SUCCESSx bypassing the initial stack element size limit. Verified that the new test catches the mutant: ViniciusCestarii@f8f42a1. ACKs for top commit: instagibbs: ACK 558e26e Tree-SHA512: 66d7bbbf286bf7e5c5762704e8c0f835c6a8026d7d604263e9debcd7e71df80506c0238b8a15cda3d4ba245d9c2bc46e79a1986d66583e571e208c8dfbe66156
This bump includes the new github mirrors for sourceware repos for: binutils https://github.com/gnutools/binutils-gdb.git gdb https://github.com/gnutools/binutils-gdb.git glibc https://github.com/gnutools/glibc.git newlib https://github.com/cygwin/cygwin.git musl https://github.com/kraj/musl.git from commit riscv-collab/riscv-gnu-toolchain@92d2391
This reverts commit b283e17.
607f220 Revert "ci: use mirror for riscv submodules" (will) 64af18f ci: bump riscv toolchain to tag 2026.08.25 (will) Pull request description: This bump includes the new github mirrors for sourceware repos for: binutils https://github.com/gnutools/binutils-gdb.git gdb https://github.com/gnutools/binutils-gdb.git glibc https://github.com/gnutools/glibc.git newlib https://github.com/cygwin/cygwin.git musl https://github.com/kraj/musl.git from commit riscv-collab/riscv-gnu-toolchain@92d2391 so that we can remove the fish.foo mirrors. ACKs for top commit: maflcko: lgtm ACK 607f220 sedited: ACK 607f220 Tree-SHA512: 627ce75b73e9b8dace6bc883d4e017753d531b083a7ac440a542656c124d5a3dc7827c7390287cb232ebf53416018e2717623d81f92c3518b00cfbff994db1d5
… help text d5e64b0 doc: note -blocknotify is not run during IBD/reindex in help text (Guillermo Fernandes) Pull request description: Adds a note to the `-blocknotify` help output that it is not executed for blocks connected during initial block download or reindexing (it only fires once the node reaches the post-init sync state). Suggested by sedited in review of #35518. ACKs for top commit: sedited: ACK d5e64b0 Tree-SHA512: 06b4a3c6354689e9846b5af6c210c3bb360b917477ac9506814c0238a1afb78392c4b6a6bbd51a4b9d98c841a1bc86686e90072155ce9a4f5e8f875cfe288c0c
…s errors b3ff9c4 iwyu: Fix warnings in `src/interfaces` and treat them as errors (Hennadii Stepanov) d564b02 iwyu: Add temporary mapping to work around upstream bug (Hennadii Stepanov) Pull request description: This PR continues the ongoing effort to enforce IWYU warnings. See [Developer Notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#using-iwyu). ACKs for top commit: maflcko: review ACK b3ff9c4 🖋 Tree-SHA512: 9359854c7b6e6ec93022947e78ef57fc68728326df874371028997a550fd1b6ef08e4b09271163b9d5c921c9e01deadf92b0eb5ae3f620d797e4f9bdea8f4727
bed46bd build: check for SetThreadDescription() at configure time (ViniciusCestarii) Pull request description: SetThreadDescription() is missing from mingw-w64 headers before 12.0.0, so the Windows cross-compile fails on distro toolchains, e.g. Ubuntu 24.04. Reported by hebasto in #35884 (comment). Check for the symbol at configure time and guard its use with a new `HAVE_SETTHREADDESCRIPTION` guard, as cmake/introspection.cmake already does for other optional symbols. This avoids having to declare a minimum mingw-w64 version: toolchains that have the symbol get OS-level thread names, older ones build fine without them. ACKs for top commit: fanquake: utACK bed46bd - could be reverted + docs updated post branch-off. hebasto: re-ACK bed46bd. Tree-SHA512: 3edbbd252fc68e976d930a8a6124746b3ba586ea58dc0720a67f8975e935057e2838bb7484d6b789771a327d2b69a092d64f335dd483f90b84a3ef290c138bfb
1ad8641 iwyu: Fix warnings in `src/init` and treat them as errors (Hennadii Stepanov) Pull request description: This PR continues the ongoing effort to enforce IWYU warnings. See [Developer Notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#using-iwyu). ACKs for top commit: maflcko: lgtm ACK 1ad8641 Tree-SHA512: d63d2f5aeac487f01012b8802aff32eb53a8b5d53b8a6c8ece2a40b8c9603402f4f9a20c0918b3007ffaee1ed38a1009698e7b80be29c6a5175517e3279db952
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.