3c8f5e48f710313de78bcbfafd09fed71890d754 ci: Treat SHA1 LLVM signing key as warning (will)
Pull request description:
The current SHA1 LLVM signing key is considered not secure since
2026-02-01T00:00:00Z which makes this run fail when downloading
packages.
See: https://github.com/llvm/llvm-project/issues/153385
Apply the fix from the issue to temporarily to treat this error as a
warning, until the upstream key can be updated.
This PR should be reverted once the upstream key is updated.
ACKs for top commit:
hebasto:
ACK 3c8f5e48f710313de78bcbfafd09fed71890d754, tested by running the "iwyu" CI job locally on Ubuntu 25.10 after burning all podman's caches.
Tree-SHA512: fbccf98bfd73cb338670f1ceea994d277d746acbc88b9b90a403d9a59d82abda0f3ba34c4d484b70926340c2d0c873259f930c36ccd4f9d18bb1d22d49ee70c4
The current SHA1 LLVM signing key is considered not secure since
2026-02-01T00:00:00Z which makes this run fail when downloading
packages.
See: https://github.com/llvm/llvm-project/issues/153385
Apply the fix from the issue to temporarily to treat this error as a
warning, until the upstream key can be updated.
This PR should be reverted once the upstream key is updated.
dfb93646093f8d71be455fc95e2e06ff73fb9211 fuzz: pull latest FuzzedDataProvider.h from upstream (b-l-u-e)
Pull request description:
Pulls down the latest version of https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/fuzzer/FuzzedDataProvider.h, after https://github.com/llvm/llvm-project/pull/177794 was merged upstream.
ACKs for top commit:
fanquake:
ACK dfb93646093f8d71be455fc95e2e06ff73fb9211 - updated the PR description.
Tree-SHA512: 36d003a1d92158537811d044bf4b79d63f0ec79e5b0da9e26a73a869c81578f9597ababcb79bb1949689db506c25341255c59979214ee28343fa08f85e2c3466
facb2aab26dffbc1e46809ac776ed43b9eaa9ad4 test: Turn ElapseSteady into SteadyClockContext (MarcoFalke)
Pull request description:
`ElapseSteady` was introduced a while back, but is only used in one place. It makes more sense if this were a context manager, so that mocktime does not leak from one test into the next.
So turn it into a context manager, rename it and allow easy time advancement via e.g. `steady_ctx += 1h`.
ACKs for top commit:
l0rinc:
ACK facb2aab26dffbc1e46809ac776ed43b9eaa9ad4
ismaelsadeeq:
utACK facb2aab26dffbc1e46809ac776ed43b9eaa9ad4
sedited:
ACK facb2aab26dffbc1e46809ac776ed43b9eaa9ad4
Tree-SHA512: 1df9cc9685d9be4d3ab8deafd99ac1a5ff752064ae54b83bacd6f44ba2c198b091558a306d49d8b1e2200ac669e95915cc792d589fb3a63b2bef7891d325a1e0
3bd98b45084d3029465110a99e2486d48944ded8 refactor: use transparent comparator for setBlockIndexCandidates lookups (joaonevess)
Pull request description:
### Rationale
This PR improves code safety by removing a `const_cast` in `src/validation.cpp`.
Currently, `setBlockIndexCandidates` stores mutable `CBlockIndex*`. However, validation logic (like `CVerifyDB`) often holds `const CBlockIndex*`. Previously, checking for existence in the set required casting away constness. While currently benign, this bypasses compiler safety checks and could mask accidental modifications in future refactors.
### Description
1. **Enable Heterogeneous Lookup:** Added `using is_transparent = void;` to `CBlockIndexWorkComparator` in `src/node/blockstorage.h`. This allows the `std::set` to natively accept `const CBlockIndex*` for lookup (utilizing C++14 heterogeneous lookup).
2. **Remove Cast:** Removed the now unnecessary `const_cast<CBlockIndex*>` in `src/validation.cpp`, allowing the compiler to strictly enforce const-correctness.
### Notes
- **Refactoring only:** No behavioral change.
- **Verification:** `validation_tests` and `blockmanager_tests` pass.
ACKs for top commit:
maflcko:
review ACK 3bd98b45084d3029465110a99e2486d48944ded8 🚪
frankomosh:
ACK 3bd98b45084d3029465110a99e2486d48944ded8. Good use of transparent comparator to eliminate `const_cast` in this specific code path.
sedited:
ACK 3bd98b45084d3029465110a99e2486d48944ded8
Tree-SHA512: 0f76bdce2a54b759dfec99633afce1e95586e62f4057ecf1e82eed1a073eb8ecb2d659ccbf28a7a139f0aa09a30f058ac6966cafdfbf1f2ee878fa2d86b2c487
51abf7d15b1da5b74d58a381ced662dc0f70b4b0 script: remove unused SCRIPT_ERR_LAST (Antoine Poinsot)
Pull request description:
It was introduced in ab9edbd6b6eb3efbca11f16fa467c3c0ef905708 and never used since. It seems it might have been intended to be exposed as part of a public library interface, which has since been superseded.
The only call site uses SCRIPT_ERR_ERROR_COUNT directly.
ACKs for top commit:
billymcbip:
tACK 51abf7d15b1da5b74d58a381ced662dc0f70b4b0
sedited:
ACK 51abf7d15b1da5b74d58a381ced662dc0f70b4b0
theStack:
ACK 51abf7d15b1da5b74d58a381ced662dc0f70b4b0
Tree-SHA512: 983b0523b2b5eba57732223af22746c9f29e4759d23366147825d1101f94a9b10c385f305d1425c439a4e29ab28f5a9245691ba6dc31a13f260d3d03b0bf1885
eeb4d2814803c09af602ca5c9810438dd5e987fb validation: follow-up nits for lock-free `IsInitialBlockDownload()` (Lőrinc)
Pull request description:
Innocent follow-up to #34253:
* Add `AssertLockHeld(cs_main)` to `ChainstateManager::UpdateIBDStatus()` given it's already annotated with `EXCLUSIVE_LOCKS_REQUIRED(cs_main)`.
* Fix outdated comment about constness of `ChainstateManager::IsInitialBlockDownload()` (compilation and build passes without it).
* And since we're touching it, we might as well mark `ChainstateManager::IsInitialBlockDownload()` as `noexcept` now.
ACKs for top commit:
davidgumberg:
crACK eeb4d28148
sedited:
ACK eeb4d2814803c09af602ca5c9810438dd5e987fb
mzumsande:
utACK eeb4d2814803c09af602ca5c9810438dd5e987fb
Tree-SHA512: 110cf5b03dc4f4cf6e61563ef69da6368e43009cf0fe1b10870cb4f55203c347444c8623aae7357d0ee5ba3f4b10da535b440a5871c9c5a4f7f8f88c2accd1f1
1bf384222323885ffafb669042d97a4fc5327586 ci, iwyu: Fix warnings in `src/univalue` 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:
review ACK 1bf384222323885ffafb669042d97a4fc5327586 🦇
sedited:
ACK 1bf384222323885ffafb669042d97a4fc5327586
Tree-SHA512: e2b7ad7b318c6c78de9a6b416a2bcb6da3ec9f4f060590db2b4f5f3a9a167891ac60c68efccea889df806f53b803c98303eed1fc24682257a9c37844ee2ee55d
07af50f7896a36a82efc19b5030779ab36302fa4 util: Drop *BSD headers in `batchpriority.cpp` (Hennadii Stepanov)
Pull request description:
Currently, there are issues with headers in `batchpriority.cpp`:
1. `SCHED_BATCH` is not defined on all supported *BSD platforms.
2. `pthread.h` is necessary on other platforms.
This PR addresses both issues and fixes other includes.
ACKs for top commit:
maflcko:
review ACK 07af50f7896a36a82efc19b5030779ab36302fa4 🤺
w0xlt:
crACK 07af50f789
Tree-SHA512: 388f627e9d9c8b80834ba562034cd6aa44ba37d9156e13cdd6cbcfa93b19fcbf3222228d671784ca363e06fda3c978778f62f4530c3eb4ae3ff96adf91c2d789
9c839aa9e3db30e2fa7d45b087a13bdd86b2a085 iwyu: Document mappings for libc symbols (Hennadii Stepanov)
91824646c58afade176d6c0003c892ceca855ba9 iwyu: Add temporary mapping to work around upstream bug (Hennadii Stepanov)
37de7d19107c350e5ff0b8fc0cb5551830bdb6cb iwyu: Drop backported mapping (Hennadii Stepanov)
Pull request description:
This PR:
1. Removes mappings that have been [backported](https://github.com/include-what-you-use/include-what-you-use/pull/1706) upstream.
2. Adds a new temporary mapping to work around upstream [issue](https://github.com/include-what-you-use/include-what-you-use/issues/1616).
3. Document the existing mappings for libc symbols.
ACKs for top commit:
maflcko:
lgtm ACK 9c839aa9e3db30e2fa7d45b087a13bdd86b2a085
sedited:
ACK 9c839aa9e3db30e2fa7d45b087a13bdd86b2a085
Tree-SHA512: 691fd9fc6798951ca1a621ee607a617ebef2ed2f43196465c78c6cd7a5ea7f0f0e876e68cd54653edf9b6762ae5fdb704469a1524a5f6f1ab8b0eedbff65cc28
efcbf794484ecc02cae05e520120df9d1aa8c93a ci, iwyu: Fix warnings in `src/zmq` and treat them as errors (Hennadii Stepanov)
Pull request description:
This PR [continues](https://github.com/bitcoin/bitcoin/pull/33725#issuecomment-3466897433) the ongoing effort to enforce IWYU warnings.
See [Developer Notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#using-iwyu).
Additionally, this adds a new include category to `src/.clang-format`.
ACKs for top commit:
maflcko:
review ACK efcbf794484ecc02cae05e520120df9d1aa8c93a 🐼
janb84:
re ACK efcbf794484ecc02cae05e520120df9d1aa8c93a
sedited:
ACK efcbf794484ecc02cae05e520120df9d1aa8c93a
Tree-SHA512: 5396719d4a9f7fff7b57be7284af5b25ff055edbaba417187e29106c9e310f19f361fbeea74e2448ef1e883a8658028762a38664858a863e5019fcb0cbb346a2
b189a345574460f10165862eca9cc40ff3337dca test: add case where `TOTAL_TRIES` is exceeded yet solution remains (yancy)
Pull request description:
Show that `CoinGrider` halts searching when the number of attempts exceeds `TOTAL_TRIES`. To do so, show that a solution is found, then add one more entry to the same set of inputs. Since the search orders by `effective_value`, the solution is constructed such that only values with the lowest `effective_value` have the least weight. Only the lowest weight values will not exceed the `max_selection_weight`. Therefore, `CoinGrinder` will not evaluate all lowest weight solutions together before exceeding `TOTAL_TRIES` since they are last found.
This test case was inspired by a similar test for `BnB` currently named `bnb_test`.
ACKs for top commit:
frankomosh:
Code review ACK b189a34
achow101:
ACK b189a345574460f10165862eca9cc40ff3337dca
murchandamus:
ACK b189a345574460f10165862eca9cc40ff3337dca
Tree-SHA512: 1df0b6e29ae219edbeed14cfa97f0ad4688d6bf97ed946719ba3c3b69e004f3dee82991578eb5aceb554914b70c5b68feff9e321283c1fc8bc0fedf08df2cb4c
552bc82b17961b86ae1964e817ba89ee7bfd985f doc: Use multipath descriptors in descriptors.md and linked test (Anurag chavan)
Pull request description:
Updates documentation and `wallet_miniscript_decaying_multisig_descriptor_psbt.py` to use single multipath descriptors with `<0;1>` syntax instead of separate external/internal descriptors.
## Changes
- **doc/descriptors.md**: Update examples (lines 70-71) to use `/<0;1>/*` multipath syntax
- **doc/descriptors.md**: Update Basic Multisig Example instructions (line 179) to use single multipath descriptor
- **test/functional/wallet_miniscript_decaying_multisig_descriptor_psbt.py**: Refactor to use single multipath descriptor pattern matching `wallet_multisig_descriptor_psbt.py`
## Implementation
- `_get_xpub()` now extracts external descriptor and converts to multipath format
- `create_multisig()` imports single descriptor that expands to receive and change addresses
- Removed fake checksums from documentation examples
- Added clear comments documenting multipath convention
Fixes#34086
ACKs for top commit:
yashbhutwala:
ACK 552bc82b17961b86ae1964e817ba89ee7bfd985f
achow101:
ACK 552bc82b17961b86ae1964e817ba89ee7bfd985f
rkrux:
lgtm tACK 552bc82
Tree-SHA512: cc99271a3955daa475242d9f4ef8f09f4c94c64e48ec4647ecfd95dceb38bb0cdd91b78ec2d5f033b449d175eaecbdda49d6c766c8a1e1a01fed93be4eb0cfc0
6f7b4323cb46687ba9df7073a9cde427985d2dfc test: remove UNKNOWN_ERROR from script_tests (Bruno Garcia)
bd31a92d671610e1173a4e6f0c761d94724441ae script: use SCRIPT_ERR_SCRIPTNUM for CScriptNum errors (Bruno Garcia)
0ca4dcd78665bc5258a2b9cfcef0dcdf971a88c3 script: add SCRIPT_ERR_SCRIPTNUM error (Bruno Garcia)
Pull request description:
When evaluating a script, the current code is bad for analyzing some errors because it returns `SCRIPT_ERR_UNKNOWN_ERROR` for errors that are clearly known.
`CScriptNum` has two well defined errors: number overflow and non-minimally encoded number. However, for both errors we return as unknown. This PR changes it by adding a new ScriptError that is used for any `CScriptNum` error.
ACKs for top commit:
achow101:
ACK 6f7b4323cb46687ba9df7073a9cde427985d2dfc
w0xlt:
ACK 6f7b4323cb
darosior:
ACK 6f7b4323cb46687ba9df7073a9cde427985d2dfc
Tree-SHA512: e656d9992251fbc95d33966fa18ce64bf714179d51ba6a7f429e5a55bc58e7fc08827e4ab71ace0dd385dac7e1feaea621b49503387793a30eae7a7e44aa6b0f
964c44cdcd6be5f39aed1aeda9c305803eb3b25f test(miniscript): Prove avoidance of stack overflow (Hodlinator)
198bbaee4959119a63b4038cd0dbb519f4daf6f0 refactor(miniscript): Destroy nodes one full subs-vector at a time (Hodlinator)
50cab8570e8f7553a94e750f66ad9228a728e72e refactor(miniscript): Remove NodeRef & MakeNodeRef() (Hodlinator)
15fb34de41cb069e2bad93a64722bdb32ff00690 refactor(miniscript): Remove superfluous unique_ptr-indirection (Hodlinator)
e55b23c170eb1a80a71e2de8b48cf8a0aebda843 refactor(miniscript): Remove Node::subs mutability (Hodlinator)
c6f798b22247bc092e72eed9e9f69a0cbaca5134 refactor(miniscript): Make fields non-const & private (Hodlinator)
22e4115312b929502574ba3681ee2c3b3fd14d96 doc(miniscript): Remove mention of shared pointers (Hodlinator)
Pull request description:
Removes one level of unnecessary indirection, which was a change that originally [aided in finding one issue](https://github.com/bitcoin/bitcoin/pull/30866#pullrequestreview-2434704657) in #30866. Simplifies the code one step further than 09a1875ad8cddeb17c19af34b8282d37fed0937e belonging to aforementioned PR.
Also adds test which verifies resistance to stack overflow when it comes to `~Node()` and `Node::Clone()`.
No observed difference when running benchmarks: ExpandDescriptor/WalletIsMineDescriptors/WalletIsMineMigratedDescriptors/WalletLoadingDescriptors.
Followup to #30866.
ACKs for top commit:
achow101:
ACK 964c44cdcd6be5f39aed1aeda9c305803eb3b25f
darosior:
Code review ACK 964c44cdcd6be5f39aed1aeda9c305803eb3b25f
l0rinc:
ACK 964c44cdcd6be5f39aed1aeda9c305803eb3b25f
Tree-SHA512: 32927e8f0f916fb70372ffd110f7ec7207d9e7a099c21c0a7482a12e96593b673c339719f4ab166ad7c086dc43767315fc1742c5b236a3facc45c4cfeb5872e9
516be10bb56db80aa95b3afbf9773ecd7f167284 wallet: Rename `RecordType::DELETE` to `RecordType::DELETE_FLAG` (Hennadii Stepanov)
Pull request description:
On Windows, the `winnt.h` header defines `DELETE` as a macro for a "Standard Access Right" bitmask (0x00010000L).
This introduces a fragile dependency on header inclusion order: if Windows headers happen to be included before the `RecordType` enum definition, the preprocessor expands `DELETE` into a numeric literal, causing syntax errors.
Rename the enumerator to `DELETE_FLAG` to remove this fragility and avoid the collision entirely.
Split from https://github.com/bitcoin/bitcoin/pull/34448.
ACKs for top commit:
maflcko:
re-lgtm ACK 516be10bb56db80aa95b3afbf9773ecd7f167284
achow101:
ACK 516be10bb56db80aa95b3afbf9773ecd7f167284
Tree-SHA512: eba054b395e18c07efb2901b28f542b042b62d85e1a798eeff35f8431530cb667fa791c47c4125cecdb689213b458ba396715495415e9b83bb322509a9376222
Currently, there are issues with headers in `batchpriority.cpp`:
1. `SCHED_BATCH` is not defined on all supported *BSD platforms.
2. `pthread.h` is necessary on other platforms.
This addresses both issues and fixes other includes.
On Windows, the `winnt.h` header defines `DELETE` as a macro for a
"Standard Access Right" bitmask (0x00010000L).
This introduces a fragile dependency on header inclusion order: if
Windows headers happen to be included before this enum definition,
the preprocessor expands `DELETE` into a numeric literal, causing
syntax errors.
Rename the enumerator to `DELETE_FLAG` to remove this fragility and
avoid the collision entirely.
4fab35cf88c048d2784fe6d71d3f83cc4e420879 miniscript: correct and_v() properties (Antoine Poinsot)
Pull request description:
`and_v()` must never be 'd'. This is not a bug fix since this was unreachable in valid Miniscripts: the first sub of an `and_v()` must be of type V, which conflicts with (i.e. never has) property 'd'.
ACKs for top commit:
sipa:
ACK 4fab35cf88c048d2784fe6d71d3f83cc4e420879. Fuzzed for 2 months worth of CPU time.
achow101:
ACK 4fab35cf88c048d2784fe6d71d3f83cc4e420879
Tree-SHA512: 8932ad2c9188747299cb9147ff097dca8d078ce7bdd0caefa71ee2724ff81d9bef836664211c2081519a45afd50c539974d67c2a3a1a42a65a3b10b1daef8cbe
d3e681bc06758fe0686cd96fcfd4a1c4c5af62b4 fuzz: Use `__AFL_SHM_ID` for naming test directories (marcofleon)
Pull request description:
During long multicore fuzzing campaigns with AFL++, stale datadirs can eventually accumulate from time outs, resulting in disk running out of space (see https://github.com/bitcoin/bitcoin/issues/28811). The easiest way to reproduce this is by running our `utxo_total_supply` target using multiple cores with AFL++ and observing the crashes that occur because of all the directories in `/tmp/test_common\ bitcoin/utxo_total_supply/`.
Fix this by using the AFL++ shared memory ID to name the test dirs and cleaning it up before each setup. This ID is unique per AFL++ instance, so multiple cores can run in parallel without conflicts.
Fixes https://github.com/bitcoin/bitcoin/issues/28811
ACKs for top commit:
maflcko:
lgtm ACK d3e681bc06758fe0686cd96fcfd4a1c4c5af62b4
dergoegge:
utACK d3e681bc06758fe0686cd96fcfd4a1c4c5af62b4
Tree-SHA512: 420373e5f8a63c84797303ba2ef6657dfe9dacf9c2f3d818524421c24681a0e984c212ecb706217d93f67c2ec16b146a2d37fddcbd6918b2e5e9f634f5e13c10
fafdae46ff0b02d93d5fcff35f1185627d11d76a test: Check that redundant verack message is ignored (MarcoFalke)
Pull request description:
The code exists and is uncovered (ref https://maflcko.github.io/b-c-cov/total.coverage/src/net_processing.cpp.gcov.html#L3795), so add a trivial test to cover it.
ACKs for top commit:
brunoerg:
ACK fafdae46ff0b02d93d5fcff35f1185627d11d76a
sedited:
ACK fafdae46ff0b02d93d5fcff35f1185627d11d76a
Tree-SHA512: 157f434c2faa16243890b2344c4ee36bc359e56c80ba8a04f0bba71e9760cf9106c38ed755ff57eff8d1957f35516d20b3d010e0ecb8633b845f5314cc0d050a
fad2876ec330dbb833905d3b2ee5753abc3bc3af ci: Always print low ccache hit rate notice (MarcoFalke)
Pull request description:
Looks like the hit rate is low, even on test changes such as https://github.com/bitcoin/bitcoin/actions/runs/21476546461/job/61867393974#step:10:3349
to make it easier to debug, unconditionally print the low hit rate notice
ACKs for top commit:
l0rinc:
ACK fad2876ec330dbb833905d3b2ee5753abc3bc3af
sedited:
ACK fad2876ec330dbb833905d3b2ee5753abc3bc3af
Tree-SHA512: 0cd85e3572e8465ec424766b1fdb6181d7e607cae991889b46cc66e5f08354772b6040a9f14c0864d36e1f38894628819a3a7458d3ec9ea32e063257177740a0
7d9e1a810239a65a153c35f0f94490560441db49 test: Verify peer usage after assumeutxo validation completes (stringintech)
0067abe153298ce9f14262a15533033e6e907f2b p2p: Allow block downloads from peers without snapshot block after assumeutxo validation (stringintech)
Pull request description:
Currently, after assumeutxo background validation finishes, the node continues to skip peers that don't have the snapshot block in their best chain until restart. This unnecessarily excludes peers from block downloads even though the background sync has completed and undo data is available.
The restriction persists because `m_chainman.CurrentChainstate().SnapshotBase()` continues to return the snapshot base block until restart, even after validation completes. Added `m_chainman.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED` check to only apply the peer restriction while background validation is ongoing.
Also added test coverage in `feature_assumeutxo.py` that verifies peers without the snapshot block can be used for block downloads after background validation completes. The test fails without this fix.
ACKs for top commit:
fjahr:
Re-ACK 7d9e1a810239a65a153c35f0f94490560441db49
achow101:
ACK 7d9e1a810239a65a153c35f0f94490560441db49
sedited:
Re-ACK 7d9e1a810239a65a153c35f0f94490560441db49
Tree-SHA512: 5515971da7bf7efc55eecdf03686f44c20c9e52dd168e7cfa119032d6a8ebccee69df7143075e4e9d0a01426cd9ae7202dce5c00919a82478ebf49a15dc0fe19
3e0fd0e4ddd894f0e7db1772f10ceaa1dddfb951 refactor: rename will_reuse_cache to reallocate_cache (Andrew Toth)
44b4ee194d3bdccd86cf5e151b2fc1479aabbb6c validation: reuse same CCoinsViewCache for every ConnectBlock call (Andrew Toth)
8fb6043231ea396aaa1165b36b082c89e10fcafd coins: introduce CCoinsViewCache::ResetGuard (Andrew Toth)
041758f5eda5725daad4ae20f66c7d19ba02d063 coins: use hashBlock setter internally for CCoinsViewCache methods (Andrew Toth)
8dd9200fc9b0d263f8f75943ce581a925d061378 coins: add Reset on CCoinsViewCache (Andrew Toth)
Pull request description:
This is the first commit of #31132, which can be merged as an independent change. It has a small benefit on its own, but will help in moving the parent PR forward.
Add a `Reset()` method to `CCoinsViewCache` that clears `cacheCoins`, `cachedCoinsUsage`, and `hashBlock` without flushing to the `base` view. This allows efficiently reusing a cache instance across multiple blocks.
Add `CCoinsViewCache::CreateResetGuard` method to return a `CCoinsViewCache::ResetGuard`. The `ResetGuard` automatically calls `Reset()` on destruction. This RAII pattern ensures the cache is always properly reset between blocks.
Add `m_connect_block_view` as a persistent `CCoinsViewCache` for `ConnectBlock`, avoiding repeated memory allocations.
ACKs for top commit:
l0rinc:
ACK 3e0fd0e4ddd894f0e7db1772f10ceaa1dddfb951
achow101:
ACK 3e0fd0e4ddd894f0e7db1772f10ceaa1dddfb951
sedited:
ACK 3e0fd0e4ddd894f0e7db1772f10ceaa1dddfb951
Tree-SHA512: a95feaa062a9eb7cf7514425a7e7adffd347cd1f7b32b4c1fefcde30002141757c184174702b3104a029dcd33194f8bd734159deebb2e668716089305b42cb00
c6ca2b85a3e6e73674e210aee4ed69c4af2848e4 validation: do not wipe utxo cache for stats/scans/snapshots (Pieter Wuille)
7099e93d0a80c65a547131d7bab977b09573310c refactor: rename `FlushStateMode::ALWAYS` to `FORCE_FLUSH` (Lőrinc)
Pull request description:
Revival of https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-3432564955 with the remaining comments applied on top
> Since #28280, the cost of a non-wiping sync of the UTXO cache is only proportional to the number of dirty entries, rather than proportional to the size of the entire cache. Because of that, there is no reason to perform a wiping flush in case the contents of the cache is still useful.
>
> Split the `FlushStateMode::ALWAYS` mode into a FORCE_SYNC (non-wiping) and a FORCE_FLUSH (wiping), and then use the former in `scantxoutset`, `gettxoutsetinfo`, snapshot creation.
(slightly updated after #30214)
ACKs for top commit:
optout21:
reACK c6ca2b85a3e6e73674e210aee4ed69c4af2848e4
cedwies:
reACK c6ca2b8 (trivial)
achow101:
ACK c6ca2b85a3e6e73674e210aee4ed69c4af2848e4
sedited:
ACK c6ca2b85a3e6e73674e210aee4ed69c4af2848e4
Tree-SHA512: f3525a85dc512db4a0a9c749ad47c0d3fa44085a121aa54cd77646260a719c71f754ec6570ae77779c0ed68a24799116f79c686e7a17ce57a26f6a598f7bf926
Newer versions of the macOS SDK, have introduced code like:
```cpp
if defined(__has_feature) && __has_feature(modules)
define USE_CLANG_TYPES 1
else
define USE_CLANG_TYPES 0
endif
if USE_CLANG_TYPES
include <sys/_types/_ptrdiff_t.h>
include <sys/_types/_size_t.h>
include <sys/_types/_va_list.h>
include <sys/_types/_wchar_t.h>
endif
```
which is currently causing compile failures due to undeclared types,
which manifest when C++ modules are enabled. Note that the usage of
"modules" in LLVM is can be a bit ambiguous, see:
https://github.com/llvm/llvm-project/issues/55891.
For now, explcitly disable cxx modules using `-fno-cxx-modules`. This
resolves the include/compilation issues.
Related discussion:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116827https://github.com/llvm/llvm-project/pull/150349https://github.com/llvm/llvm-project/issues/57432
Add m_connect_block_view to ChainState's CoinsViews.
Call CreateResetGuard inside ConnectTip to ensure the view
is Reset after each block, avoiding repeated memory allocations.
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
CCoinsViewCache::CreateResetGuard returns a guard that calls
Reset on the cache when the guard goes out of scope.
This RAII pattern ensures the cache is always properly reset
when it leaves current scope.
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
Co-authored-by: sedited <seb.kung@gmail.com>
Add a Reset() method to CCoinsViewCache that clears cacheCoins,
cachedCoinsUsage, and hashBlock without flushing to the base view.
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
Co-authored-by: sedited <seb.kung@gmail.com>
Use the AFL++ shared memory ID environment variable to create
a deterministic datadir path. This prevents accumulation of stale
directories after a fuzz iteration crashes or times out. During
long fuzz campaigns, this accumulation has occasionally resulted
in running out of disk space.
2845f10a2be0fee13b2772d24e948052243782b8 test: extend FreeBSD ephemeral port range fix to P2P listeners (node)
34bed0ed8c449a3834927cec3447dbe6c74edf3d test: use IP_PORTRANGE_HIGH on FreeBSD for dynamic port allocation (woltx)
Pull request description:
Reopening #34336. I’ve now tested it on FreeBSD and confirmed it works.
On FreeBSD, the default ephemeral port range (10000-65535) overlaps with the test framework's static port range (11000-26000), possibly causing intermittent "address already in use" failures when tests use dynamic port allocation (`port=0`).
This PR adds a helper that sets `IP_PORTRANGE_HIGH` via `setsockopt()` before binding, requesting ports from 49152-65535 instead, which avoids the overlap, as suggested in https://github.com/bitcoin/bitcoin/issues/34331#issuecomment-3767161843 by @maflcko .
From FreeBSD's [sys/netinet/in.h](https://cgit.freebsd.org/src/tree/sys/netinet/in.h):
```c
#define IP_PORTRANGE 19
#define IP_PORTRANGE_HIGH 1
#define IPPORT_EPHEMERALFIRST 10000 /* default range start */
#define IPPORT_HIFIRSTAUTO 49152 /* high range start */
```
See also: FreeBSD https://man.freebsd.org/cgi/man.cgi?query=ip&sektion=4 man page.
Fixes#34331
ACKs for top commit:
vasild:
ACK 2845f10a2be0fee13b2772d24e948052243782b8
hebasto:
ACK 2845f10a2be0fee13b2772d24e948052243782b8, I have reviewed the code and it looks OK.
Tree-SHA512: ce501ce3e8a4023e07bad572df2b85d6829becf133813e4529aebba83e4eba59fa8b48e9d2197ebbb226adaf3054fad720775a787244d6b38c0078ee086102f4
Add `AssertLockHeld(cs_main)` to `ChainstateManager::UpdateIBDStatus()` given `EXCLUSIVE_LOCKS_REQUIRED(cs_main)`.
Fix outdated comment about constness of `ChainstateManager::IsInitialBlockDownload()` (build passes without it).
And since we're touching it, we might as well mark `ChainstateManager::IsInitialBlockDownload()` as `noexcept` now.
Follow-up to #34253.
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
fad042235bd6054d99d3f5a07529276b0138b484 refactor: Remove remaining std::bind, check via clang-tidy (MarcoFalke)
Pull request description:
`std::bind` has many issues:
* It is verbose in a meaningless way
* Overriden args are silently accepted and dropped at runtime without a compile error. Same for accidental duplicates.
One could use `std::bind_front` similar to commit fa267551c4eaef577db92e248c4b6d31d0c8bc77. Though, I think the remaining cases are better off with lambdas.
So do that here, and enable the `modernize-avoid-bind` clang-tidy rule to avoid `std::bind` bugs in the future.
ACKs for top commit:
fjahr:
Code review ACK fad042235bd6054d99d3f5a07529276b0138b484
purpleKarrot:
Code review ACK fad042235bd6054d99d3f5a07529276b0138b484
Tree-SHA512: 38b17e26eda3ae47d84a8c34298309dc1eeb4ed434fda58b5803ef031c4c2edfb17222f5208f37af727bf340e32b37c7f81784f461d2b65fbc6227f3cd53eea4
e770392084aa52e5568cf001da4d537fda1d71b3 test: addrman: test self-announcement time penalty handling (Bruno Garcia)
Pull request description:
This PR adds a test case for addrman that verifies that addresses announcing themselves (addr == source) are exempt from time penalties, while addresses announced by others receive the expected penalty.
It fixes the following mutant (https://corecheck.dev/mutation/src/addrman.cpp#L561):
```diff
diff --git a/src/addrman.cpp b/src/addrman.cpp
index 206b54118e..c6a045fd8d 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -558,7 +558,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, std::c
AddrInfo* pinfo = Find(addr, &nId);
// Do not set a penalty for a source's self-announcement
- if (addr == source) {
+ if (addr != source) {
time_penalty = 0s;
}
```
ACKs for top commit:
maflcko:
review ACK e770392084aa52e5568cf001da4d537fda1d71b3 🐤
achow101:
ACK e770392084aa52e5568cf001da4d537fda1d71b3
fjahr:
Code review ACK e770392084aa52e5568cf001da4d537fda1d71b3
naiyoma:
tACK e770392084aa52e5568cf001da4d537fda1d71b3
Tree-SHA512: ec029d1e1e979f91840af944984cad530a1ce9a0eceb123230817f0ef3b9ad47253eebc4c953d350de2d904b59496fcd4757123c8bd63cf0e09c3581da48fff8
2ee7f9b259059d59e127852ea898b58183604b46 coins: assume `GetCoin` only returns unspent coins (Andrew Toth)
eec551aaf1dff4cccc15e486d5618a8a44d8314c fuzz: keep `coinscache_sim` backend free of spent coins (Andrew Toth)
3e4155fcefe0aafcc9cb84640e303e05477605a3 test: do not return spent coins from `CCoinsViewTest::GetCoin` (Andrew Toth)
ee1e40f58000921e95f08bcb199a452eb5c4d9b2 txdb: assert `CCoinsViewDB::GetCoin` only returns unspent coins (Lőrinc)
Pull request description:
This PR is split out from #33018 to keep that PR focused on removing the `FRESH-but-not-DIRTY` cache state.
### Problem
`::GetCoin()` is an interface for querying the UTXO set, so production implementations should only ever return unspent coins. Tests should mimic this to provide useful feedback.
### Fix:
* Add a fail-fast assertion that `CCoinsViewDB::GetCoin()` never returns a spent coin.
* Align unit tests and fuzz simulations with the production `GetCoin()` contract by never returning spent coins.
* Replace the unreachable “spent coin returned by parent” handling in `CCoinsViewCache::FetchCoin()` with `Assert(!coin.IsSpent())`, drop outdated `spent+FRESH` docs, and tighten `SanityCheck()` invariants.
Behavior is unchanged, it just aligns our tests to exercise valid states.
ACKs for top commit:
andrewtoth:
re-ACK 2ee7f9b259059d59e127852ea898b58183604b46
optout21:
crACK 2ee7f9b259059d59e127852ea898b58183604b46
achow101:
ACK 2ee7f9b259059d59e127852ea898b58183604b46
w0xlt:
reACK 2ee7f9b259059d59e127852ea898b58183604b46
Tree-SHA512: be21cc09690410fc04ca25e1ba47aae6186bc037e413b3bb1e6e9a04e6364cbfac5a2fcdc49b638fec848cd29243fab0cc0581b9923f34fafe8366828f690ed4
and_v() must never be 'd'. This is not a bug fix since this was
unreachable in valid Miniscripts: the first sub of an and_v() must be of
type V, which conflicts with (i.e. never has) property 'd'.
It was introduced in ab9edbd6b6eb3efbca11f16fa467c3c0ef905708 and never
used since. It seems it might have been intended to be exposed as part
of a public library interface, which has since been superseded.
The only call site uses SCRIPT_ERR_ERROR_COUNT directly.
